Skip to content

Commit bb35c48

Browse files
committed
fix(messaging): harden Google Chat approval activation
Signed-off-by: Ho Lim <subhoya@gmail.com>
1 parent 405883a commit bb35c48

4 files changed

Lines changed: 155 additions & 3 deletions

File tree

docs/manage-sandboxes/set-up-google-chat.mdx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,17 @@ The re-add flow prompts for the service-account JSON again when it is not alread
172172
## Verify the Channel
173173

174174
After the rebuild, send a direct message from an allowed or paired account and confirm that OpenClaw replies.
175+
If an unknown sender receives a pairing code, approve it through the registered OpenClaw sandbox:
176+
177+
```bash
178+
nemoclaw my-assistant exec -- openclaw pairing approve googlechat <code>
179+
```
180+
181+
After OpenClaw commits the sender to its owner allowlist, NemoClaw verifies the mutable config permissions and automatically restarts the managed gateway.
182+
An exit status of `0` means activation completed, so the sender's next message should receive a reply without another manual restart.
183+
If permission cleanup or gateway restart fails after the approval commits, `exec` exits with status `1`, reports that the approval was not rolled back, and prints the recovery command.
184+
Correct any reported permission problem, then run `nemoclaw my-assistant gateway restart` before testing the next message; do not submit the pairing code again unless OpenClaw reports that it was not accepted.
185+
175186
If the webhook returns an error, verify that the public endpoint still ends in `/googlechat`, the dedicated tunnel and webhook proxy are running, and the Google Chat API configuration contains the exact same URL.
176187

177188
Refer to [Manage Messaging Channels](manage-messaging-channels) to stop, start, or remove Google Chat after setup.

docs/reference/commands.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,6 +1321,11 @@ When cleanup succeeds, `exec` returns the remote command's exit code.
13211321
If cleanup cannot inspect, restore, or verify that contract, it fails closed and prints `OpenClaw permission cleanup failed (...)` to `stderr`.
13221322
In that case, `exec` returns the cleanup failure instead of the remote command's status.
13231323

1324+
For a registered OpenClaw sandbox, a successful direct `openclaw pairing approve googlechat <code>` command also restarts the managed gateway after cleanup so the new sender allowlist applies to the next message.
1325+
If cleanup or restart fails after the approval commits, `exec` exits with status `1` and reports that the approval was not rolled back.
1326+
Correct any reported cleanup problem, then run `$$nemoclaw <name> gateway restart` before testing the next message.
1327+
NemoClaw does not apply this automatic restart to unregistered or non-OpenClaw sandboxes.
1328+
13241329
</AgentOnly>
13251330
<AgentOnly variant="hermes">
13261331

@@ -2148,6 +2153,10 @@ After the remote command exits, NemoClaw verifies and, when needed, restores the
21482153
When cleanup succeeds, `exec` preserves the remote command's exit code.
21492154
When cleanup fails closed, `exec` returns the cleanup failure and reports both statuses on `stderr`.
21502155

2156+
A successful direct `openclaw pairing approve googlechat <code>` command in a registered OpenClaw sandbox restarts the managed gateway after cleanup, so the new sender allowlist applies to the next message.
2157+
If the approval commits but cleanup or restart fails, `exec` exits with status `1`, reports that the approval was not rolled back, and directs you to run `$$nemoclaw <name> gateway restart` after correcting any cleanup problem.
2158+
Unregistered and non-OpenClaw sandboxes do not receive this automatic restart.
2159+
21512160
</AgentOnly>
21522161
<AgentOnly variant="hermes">
21532162

src/lib/actions/sandbox/exec-googlechat-pairing-restart.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ function depsFor(status: number, restartGateway = vi.fn(() => ({ ok: true }))):
2727
run: () => ({ status }),
2828
cleanupDeps: CLEANUP_SKIPPED,
2929
restartGateway,
30+
resolveSandboxAgent: () => "openclaw",
3031
policyHint: {
3132
now: () => 1_000,
3233
probeLogs: () => "",
@@ -139,6 +140,7 @@ describe("Google Chat pairing approval gateway activation (#8553)", () => {
139140

140141
it("does not restart when post-command config cleanup fails", async () => {
141142
const restartGateway = vi.fn(() => ({ ok: true }));
143+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
142144
const deps = depsFor(0, restartGateway);
143145
deps.cleanupDeps = {
144146
getSandbox: () => {
@@ -155,17 +157,51 @@ describe("Google Chat pairing approval gateway activation (#8553)", () => {
155157

156158
expect(restartGateway).not.toHaveBeenCalled();
157159
expect(exitCode).toBe(1);
160+
expect(errorSpy).toHaveBeenCalledWith(
161+
expect.stringContaining("pairing approval committed for 'alpha'"),
162+
);
163+
expect(errorSpy).toHaveBeenCalledWith(
164+
expect.stringContaining("nemoclaw alpha gateway restart"),
165+
);
158166
});
159167

160168
it("fails the public command when activation restart fails", async () => {
161169
const restartGateway = vi.fn(() => ({ ok: false }));
170+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
171+
const exitCode = await runAndCaptureExit(
172+
["openclaw", "pairing", "approve", "googlechat", "ABCD1234"],
173+
depsFor(0, restartGateway),
174+
);
175+
176+
expect(restartGateway).toHaveBeenCalledOnce();
177+
expect(exitCode).toBe(1);
178+
expect(errorSpy).toHaveBeenCalledWith(
179+
expect.stringContaining("pairing approval committed for 'alpha'"),
180+
);
181+
expect(errorSpy).toHaveBeenCalledWith(
182+
expect.stringContaining("nemoclaw alpha gateway restart"),
183+
);
184+
});
185+
186+
it("reports a controlled partial commit when the activation restart throws", async () => {
187+
const restartGateway = vi.fn(() => {
188+
throw new Error("supervisor transport unavailable");
189+
});
190+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
191+
162192
const exitCode = await runAndCaptureExit(
163193
["openclaw", "pairing", "approve", "googlechat", "ABCD1234"],
164194
depsFor(0, restartGateway),
165195
);
166196

167197
expect(restartGateway).toHaveBeenCalledOnce();
168198
expect(exitCode).toBe(1);
199+
expect(errorSpy).toHaveBeenCalledWith(
200+
expect.stringContaining("approval was not rolled back"),
201+
);
202+
expect(errorSpy).toHaveBeenCalledWith(
203+
expect.stringContaining("nemoclaw alpha gateway restart"),
204+
);
169205
});
170206

171207
it("does not restart after a failed approval", async () => {
@@ -189,4 +225,55 @@ describe("Google Chat pairing approval gateway activation (#8553)", () => {
189225
expect(restartGateway).not.toHaveBeenCalled();
190226
expect(exitCode).toBe(0);
191227
});
228+
229+
it.each(["hermes", "custom-agent"])(
230+
"does not restart a recorded non-OpenClaw %s sandbox",
231+
async (agent) => {
232+
const restartGateway = vi.fn(() => ({ ok: true }));
233+
const deps = depsFor(0, restartGateway);
234+
deps.resolveSandboxAgent = () => agent;
235+
236+
const exitCode = await runAndCaptureExit(
237+
["openclaw", "pairing", "approve", "googlechat", "ABCD1234"],
238+
deps,
239+
);
240+
241+
expect(restartGateway).not.toHaveBeenCalled();
242+
expect(exitCode).toBe(0);
243+
},
244+
);
245+
246+
it("does not restart an unregistered sandbox", async () => {
247+
const restartGateway = vi.fn(() => ({ ok: true }));
248+
const deps = depsFor(0, restartGateway);
249+
deps.resolveSandboxAgent = () => null;
250+
251+
const exitCode = await runAndCaptureExit(
252+
["openclaw", "pairing", "approve", "googlechat", "ABCD1234"],
253+
deps,
254+
);
255+
256+
expect(restartGateway).not.toHaveBeenCalled();
257+
expect(exitCode).toBe(0);
258+
});
259+
260+
it("fails closed when the recorded sandbox identity cannot be read", async () => {
261+
const restartGateway = vi.fn(() => ({ ok: true }));
262+
const deps = depsFor(0, restartGateway);
263+
deps.resolveSandboxAgent = () => {
264+
throw new Error("registry unavailable");
265+
};
266+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
267+
268+
const exitCode = await runAndCaptureExit(
269+
["openclaw", "pairing", "approve", "googlechat", "ABCD1234"],
270+
deps,
271+
);
272+
273+
expect(restartGateway).not.toHaveBeenCalled();
274+
expect(exitCode).toBe(1);
275+
expect(errorSpy).toHaveBeenCalledWith(
276+
expect.stringContaining("pairing approval committed for 'alpha'"),
277+
);
278+
});
192279
});

src/lib/actions/sandbox/exec.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export type SandboxExecOptions = {
2929

3030
export type SandboxExecGatewayRestart = (sandboxName: string) => { ok: boolean };
3131

32+
export type SandboxExecAgentResolver = (sandboxName: string) => string | null;
33+
3234
type SpawnLikeResult = {
3335
status: number | null;
3436
signal?: NodeJS.Signals | null;
@@ -368,6 +370,8 @@ export type ExecSandboxDeps = {
368370
cleanupDeps?: SandboxExecCleanupDeps;
369371
/** Activate config written by a successful direct Google Chat pairing approval. */
370372
restartGateway?: SandboxExecGatewayRestart;
373+
/** Resolve the sandbox's recorded agent before applying agent-specific post-exec effects. */
374+
resolveSandboxAgent?: SandboxExecAgentResolver;
371375
/** Select the sandbox's owning gateway before the exec talks to OpenShell. */
372376
selectGateway?: (sandboxName: string) => GatewaySelectResult;
373377
};
@@ -390,6 +394,24 @@ function defaultRestartGateway(sandboxName: string): { ok: boolean } {
390394
return defaultInferenceGatewayRestart(sandboxName);
391395
}
392396

397+
function defaultResolveSandboxAgent(sandboxName: string): string | null {
398+
const entry = (
399+
require("../../state/registry") as typeof import("../../state/registry")
400+
).getSandbox(sandboxName);
401+
if (!entry) return null;
402+
return entry.agent ?? "openclaw";
403+
}
404+
405+
function googleChatPairingActivationFailureMessage(
406+
cliName: string,
407+
sandboxName: string,
408+
): string {
409+
return (
410+
` Google Chat pairing approval committed for '${sandboxName}', but managed gateway activation failed. ` +
411+
`The approval was not rolled back. Run '${cliName} ${sandboxName} gateway restart' before testing the next message.`
412+
);
413+
}
414+
393415
export async function execSandbox(
394416
sandboxName: string,
395417
command: readonly string[],
@@ -460,9 +482,32 @@ export async function execSandbox(
460482
}
461483
await emitPolicyDenialHint(completion);
462484
let exitCode = completion.code;
463-
if (exitCode === 0 && isGoogleChatPairingApproval(command)) {
464-
const restart = (deps.restartGateway ?? defaultRestartGateway)(sandboxName);
465-
if (!restart.ok) exitCode = 1;
485+
const googleChatApprovalCommitted =
486+
completion.commandCode === 0 && isGoogleChatPairingApproval(command);
487+
if (googleChatApprovalCommitted && completion.cleanupError) {
488+
console.error(googleChatPairingActivationFailureMessage(CLI_NAME, sandboxName));
489+
}
490+
if (exitCode === 0 && googleChatApprovalCommitted) {
491+
let recordedAgent: string | null;
492+
try {
493+
recordedAgent = (deps.resolveSandboxAgent ?? defaultResolveSandboxAgent)(sandboxName);
494+
} catch {
495+
console.error(googleChatPairingActivationFailureMessage(CLI_NAME, sandboxName));
496+
process.exit(1);
497+
}
498+
if (recordedAgent === "openclaw") {
499+
let restartSucceeded = false;
500+
try {
501+
restartSucceeded = (deps.restartGateway ?? defaultRestartGateway)(sandboxName).ok;
502+
} catch {
503+
// The approval already committed inside OpenClaw. Convert restart
504+
// exceptions into the same explicit partial-commit recovery contract.
505+
}
506+
if (!restartSucceeded) {
507+
console.error(googleChatPairingActivationFailureMessage(CLI_NAME, sandboxName));
508+
exitCode = 1;
509+
}
510+
}
466511
}
467512
process.exit(exitCode);
468513
}

0 commit comments

Comments
 (0)