mirror of https://github.com/openclaw/openclaw.git
fix(gateway): gate internal command persistence mutations
This commit is contained in:
parent
81445a9010
commit
09faed6bd8
|
|
@ -281,6 +281,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Gateway/bonjour: suppress the non-fatal `@homebridge/ciao` IPv4-loss assertion during interface churn so WiFi/VPN/sleep-wake changes no longer take down the gateway. (#38628, #47159, #52431)
|
||||
- Browser/launch: stop forcing an extra blank tab on browser launch so managed browser startup no longer opens an unwanted empty page. (#52451) Thanks @rogerdigital.
|
||||
- ACP/Codex session replay: preserve hidden assistant thinking when loading or rebinding existing ACP sessions so stored thought chunks do not replay into visible assistant text. Thanks @vincentkoc.
|
||||
- Gateway/commands: keep internal `chat.send` slash-command UX while requiring `operator.admin` before internal callers can persist `/exec` defaults or mutate `phone-control` node policy through `/phone arm|disarm`.
|
||||
|
||||
### Breaking
|
||||
|
||||
|
|
|
|||
|
|
@ -106,4 +106,92 @@ describe("phone-control plugin", () => {
|
|||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("blocks internal operator.write callers from mutating phone control", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-phone-control-test-"));
|
||||
try {
|
||||
let config: Record<string, unknown> = {
|
||||
gateway: {
|
||||
nodes: {
|
||||
allowCommands: [],
|
||||
denyCommands: ["calendar.add", "contacts.add", "reminders.add", "sms.send"],
|
||||
},
|
||||
},
|
||||
};
|
||||
const writeConfigFile = vi.fn(async (next: Record<string, unknown>) => {
|
||||
config = next;
|
||||
});
|
||||
|
||||
let command: OpenClawPluginCommandDefinition | undefined;
|
||||
registerPhoneControl.register(
|
||||
createApi({
|
||||
stateDir,
|
||||
getConfig: () => config,
|
||||
writeConfig: writeConfigFile,
|
||||
registerCommand: (nextCommand) => {
|
||||
command = nextCommand;
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
if (!command) {
|
||||
throw new Error("phone-control plugin did not register its command");
|
||||
}
|
||||
|
||||
const res = await command.handler({
|
||||
...createCommandContext("arm writes 30s"),
|
||||
channel: "webchat",
|
||||
gatewayClientScopes: ["operator.write"],
|
||||
});
|
||||
|
||||
expect(String(res?.text ?? "")).toContain("requires operator.admin");
|
||||
expect(writeConfigFile).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("allows internal operator.admin callers to mutate phone control", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-phone-control-test-"));
|
||||
try {
|
||||
let config: Record<string, unknown> = {
|
||||
gateway: {
|
||||
nodes: {
|
||||
allowCommands: [],
|
||||
denyCommands: ["calendar.add", "contacts.add", "reminders.add", "sms.send"],
|
||||
},
|
||||
},
|
||||
};
|
||||
const writeConfigFile = vi.fn(async (next: Record<string, unknown>) => {
|
||||
config = next;
|
||||
});
|
||||
|
||||
let command: OpenClawPluginCommandDefinition | undefined;
|
||||
registerPhoneControl.register(
|
||||
createApi({
|
||||
stateDir,
|
||||
getConfig: () => config,
|
||||
writeConfig: writeConfigFile,
|
||||
registerCommand: (nextCommand) => {
|
||||
command = nextCommand;
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
if (!command) {
|
||||
throw new Error("phone-control plugin did not register its command");
|
||||
}
|
||||
|
||||
const res = await command.handler({
|
||||
...createCommandContext("arm writes 30s"),
|
||||
channel: "webchat",
|
||||
gatewayClientScopes: ["operator.admin"],
|
||||
});
|
||||
|
||||
expect(String(res?.text ?? "")).toContain("sms.send");
|
||||
expect(writeConfigFile).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -358,6 +358,11 @@ export default definePluginEntry({
|
|||
}
|
||||
|
||||
if (action === "disarm") {
|
||||
if (ctx.channel === "webchat" && !ctx.gatewayClientScopes?.includes("operator.admin")) {
|
||||
return {
|
||||
text: "⚠️ /phone disarm requires operator.admin for internal gateway callers.",
|
||||
};
|
||||
}
|
||||
const res = await disarmNow({
|
||||
api,
|
||||
stateDir,
|
||||
|
|
@ -375,6 +380,11 @@ export default definePluginEntry({
|
|||
}
|
||||
|
||||
if (action === "arm") {
|
||||
if (ctx.channel === "webchat" && !ctx.gatewayClientScopes?.includes("operator.admin")) {
|
||||
return {
|
||||
text: "⚠️ /phone arm requires operator.admin for internal gateway callers.",
|
||||
};
|
||||
}
|
||||
const group = parseGroup(tokens[1]);
|
||||
if (!group) {
|
||||
return { text: `Usage: /phone arm <group> [duration]\nGroups: ${formatGroupList()}` };
|
||||
|
|
|
|||
|
|
@ -86,6 +86,8 @@ export async function applyInlineDirectivesFastLane(
|
|||
currentVerboseLevel,
|
||||
currentReasoningLevel,
|
||||
currentElevatedLevel,
|
||||
surface: ctx.Surface,
|
||||
gatewayClientScopes: ctx.GatewayClientScopes,
|
||||
});
|
||||
|
||||
if (sessionEntry?.providerOverride) {
|
||||
|
|
|
|||
|
|
@ -18,9 +18,11 @@ import { maybeHandleModelDirectiveInfo } from "./directive-handling.model.js";
|
|||
import type { HandleDirectiveOnlyParams } from "./directive-handling.params.js";
|
||||
import { maybeHandleQueueDirective } from "./directive-handling.queue-validation.js";
|
||||
import {
|
||||
canPersistInternalExecDirective,
|
||||
formatDirectiveAck,
|
||||
formatElevatedRuntimeHint,
|
||||
formatElevatedUnavailableText,
|
||||
formatInternalExecPersistenceDeniedText,
|
||||
enqueueModeSwitchEvents,
|
||||
withOptions,
|
||||
} from "./directive-handling.shared.js";
|
||||
|
|
@ -92,6 +94,10 @@ export async function handleDirectiveOnly(
|
|||
sessionKey: params.sessionKey,
|
||||
}).sandboxed;
|
||||
const shouldHintDirectRuntime = directives.hasElevatedDirective && !runtimeIsSandboxed;
|
||||
const allowInternalExecPersistence = canPersistInternalExecDirective({
|
||||
surface: params.surface,
|
||||
gatewayClientScopes: params.gatewayClientScopes,
|
||||
});
|
||||
|
||||
const modelInfo = await maybeHandleModelDirectiveInfo({
|
||||
directives,
|
||||
|
|
@ -344,7 +350,7 @@ export async function handleDirectiveOnly(
|
|||
elevatedChanged ||
|
||||
(directives.elevatedLevel !== prevElevatedLevel && directives.elevatedLevel !== undefined);
|
||||
}
|
||||
if (directives.hasExecDirective && directives.hasExecOptions) {
|
||||
if (directives.hasExecDirective && directives.hasExecOptions && allowInternalExecPersistence) {
|
||||
if (directives.execHost) {
|
||||
sessionEntry.execHost = directives.execHost;
|
||||
}
|
||||
|
|
@ -453,7 +459,7 @@ export async function handleDirectiveOnly(
|
|||
parts.push(formatElevatedRuntimeHint());
|
||||
}
|
||||
}
|
||||
if (directives.hasExecDirective && directives.hasExecOptions) {
|
||||
if (directives.hasExecDirective && directives.hasExecOptions && allowInternalExecPersistence) {
|
||||
const execParts: string[] = [];
|
||||
if (directives.execHost) {
|
||||
execParts.push(`host=${directives.execHost}`);
|
||||
|
|
@ -471,6 +477,9 @@ export async function handleDirectiveOnly(
|
|||
parts.push(formatDirectiveAck(`Exec defaults set (${execParts.join(", ")}).`));
|
||||
}
|
||||
}
|
||||
if (directives.hasExecDirective && directives.hasExecOptions && !allowInternalExecPersistence) {
|
||||
parts.push(formatDirectiveAck(formatInternalExecPersistenceDeniedText()));
|
||||
}
|
||||
if (shouldDowngradeXHigh) {
|
||||
parts.push(
|
||||
`Thinking level set to high (xhigh not supported for ${resolvedProvider}/${resolvedModel}).`,
|
||||
|
|
|
|||
|
|
@ -645,4 +645,91 @@ describe("handleDirectiveOnly model persist behavior (fixes #1435)", () => {
|
|||
expect(sessionEntry.thinkingLevel).toBe("off");
|
||||
expect(sessionStore["agent:main:dm:1"]?.thinkingLevel).toBe("off");
|
||||
});
|
||||
|
||||
it("blocks internal operator.write exec persistence in directive-only handling", async () => {
|
||||
const directives = parseInlineDirectives(
|
||||
"/exec host=node security=allowlist ask=always node=worker-1",
|
||||
);
|
||||
const sessionEntry = createSessionEntry();
|
||||
const sessionStore = { [sessionKey]: sessionEntry };
|
||||
const result = await handleDirectiveOnly(
|
||||
createHandleParams({
|
||||
directives,
|
||||
sessionEntry,
|
||||
sessionStore,
|
||||
surface: "webchat",
|
||||
gatewayClientScopes: ["operator.write"],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result?.text).toContain("operator.admin");
|
||||
expect(sessionEntry.execHost).toBeUndefined();
|
||||
expect(sessionEntry.execSecurity).toBeUndefined();
|
||||
expect(sessionEntry.execAsk).toBeUndefined();
|
||||
expect(sessionEntry.execNode).toBeUndefined();
|
||||
});
|
||||
|
||||
it("allows internal operator.admin exec persistence in directive-only handling", async () => {
|
||||
const directives = parseInlineDirectives(
|
||||
"/exec host=node security=allowlist ask=always node=worker-1",
|
||||
);
|
||||
const sessionEntry = createSessionEntry();
|
||||
const sessionStore = { [sessionKey]: sessionEntry };
|
||||
const result = await handleDirectiveOnly(
|
||||
createHandleParams({
|
||||
directives,
|
||||
sessionEntry,
|
||||
sessionStore,
|
||||
surface: "webchat",
|
||||
gatewayClientScopes: ["operator.admin"],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result?.text).toContain("Exec defaults set");
|
||||
expect(sessionEntry.execHost).toBe("node");
|
||||
expect(sessionEntry.execSecurity).toBe("allowlist");
|
||||
expect(sessionEntry.execAsk).toBe("always");
|
||||
expect(sessionEntry.execNode).toBe("worker-1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("persistInlineDirectives internal exec scope gate", () => {
|
||||
it("skips exec persistence for internal operator.write callers", async () => {
|
||||
const allowedModelKeys = new Set(["anthropic/claude-opus-4-5", "openai/gpt-4o"]);
|
||||
const directives = parseInlineDirectives(
|
||||
"/exec host=node security=allowlist ask=always node=worker-1",
|
||||
);
|
||||
const sessionEntry = {
|
||||
sessionId: "s1",
|
||||
updatedAt: Date.now(),
|
||||
} as SessionEntry;
|
||||
const sessionStore = { "agent:main:main": sessionEntry };
|
||||
|
||||
await persistInlineDirectives({
|
||||
directives,
|
||||
cfg: baseConfig(),
|
||||
sessionEntry,
|
||||
sessionStore,
|
||||
sessionKey: "agent:main:main",
|
||||
storePath: "/tmp/sessions.json",
|
||||
elevatedEnabled: true,
|
||||
elevatedAllowed: true,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-5",
|
||||
aliasIndex: baseAliasIndex(),
|
||||
allowedModelKeys,
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-5",
|
||||
initialModelLabel: "anthropic/claude-opus-4-5",
|
||||
formatModelSwitchEvent: (label) => `Switched to ${label}`,
|
||||
agentCfg: undefined,
|
||||
surface: "webchat",
|
||||
gatewayClientScopes: ["operator.write"],
|
||||
});
|
||||
|
||||
expect(sessionEntry.execHost).toBeUndefined();
|
||||
expect(sessionEntry.execSecurity).toBeUndefined();
|
||||
expect(sessionEntry.execAsk).toBeUndefined();
|
||||
expect(sessionEntry.execNode).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ export type HandleDirectiveOnlyParams = HandleDirectiveOnlyCoreParams & {
|
|||
currentReasoningLevel?: ReasoningLevel;
|
||||
currentElevatedLevel?: ElevatedLevel;
|
||||
surface?: string;
|
||||
gatewayClientScopes?: string[];
|
||||
};
|
||||
|
||||
export type ApplyInlineDirectivesFastLaneParams = HandleDirectiveOnlyCoreParams & {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,10 @@ import { applyVerboseOverride } from "../../sessions/level-overrides.js";
|
|||
import { applyModelOverrideToSessionEntry } from "../../sessions/model-overrides.js";
|
||||
import { resolveModelSelectionFromDirective } from "./directive-handling.model-selection.js";
|
||||
import type { InlineDirectives } from "./directive-handling.parse.js";
|
||||
import { enqueueModeSwitchEvents } from "./directive-handling.shared.js";
|
||||
import {
|
||||
canPersistInternalExecDirective,
|
||||
enqueueModeSwitchEvents,
|
||||
} from "./directive-handling.shared.js";
|
||||
import type { ElevatedLevel, ReasoningLevel } from "./directives.js";
|
||||
|
||||
export async function persistInlineDirectives(params: {
|
||||
|
|
@ -37,6 +40,8 @@ export async function persistInlineDirectives(params: {
|
|||
initialModelLabel: string;
|
||||
formatModelSwitchEvent: (label: string, alias?: string) => string;
|
||||
agentCfg: NonNullable<OpenClawConfig["agents"]>["defaults"] | undefined;
|
||||
surface?: string;
|
||||
gatewayClientScopes?: string[];
|
||||
}): Promise<{ provider: string; model: string; contextTokens: number }> {
|
||||
const {
|
||||
directives,
|
||||
|
|
@ -56,6 +61,10 @@ export async function persistInlineDirectives(params: {
|
|||
agentCfg,
|
||||
} = params;
|
||||
let { provider, model } = params;
|
||||
const allowInternalExecPersistence = canPersistInternalExecDirective({
|
||||
surface: params.surface,
|
||||
gatewayClientScopes: params.gatewayClientScopes,
|
||||
});
|
||||
const activeAgentId = sessionKey
|
||||
? resolveSessionAgentId({ sessionKey, config: cfg })
|
||||
: resolveDefaultAgentId(cfg);
|
||||
|
|
@ -110,7 +119,7 @@ export async function persistInlineDirectives(params: {
|
|||
(directives.elevatedLevel !== prevElevatedLevel && directives.elevatedLevel !== undefined);
|
||||
updated = true;
|
||||
}
|
||||
if (directives.hasExecDirective && directives.hasExecOptions) {
|
||||
if (directives.hasExecDirective && directives.hasExecOptions && allowInternalExecPersistence) {
|
||||
if (directives.execHost) {
|
||||
sessionEntry.execHost = directives.execHost;
|
||||
updated = true;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { formatCliCommand } from "../../cli/command-format.js";
|
||||
import { SYSTEM_MARK, prefixSystemMessage } from "../../infra/system-message.js";
|
||||
import { isInternalMessageChannel } from "../../utils/message-channel.js";
|
||||
import type { ElevatedLevel, ReasoningLevel } from "./directives.js";
|
||||
|
||||
export const formatDirectiveAck = (text: string): string => {
|
||||
|
|
@ -13,6 +14,20 @@ export const withOptions = (line: string, options: string) =>
|
|||
export const formatElevatedRuntimeHint = () =>
|
||||
`${SYSTEM_MARK} Runtime is direct; sandboxing does not apply.`;
|
||||
|
||||
export const formatInternalExecPersistenceDeniedText = () =>
|
||||
"Exec defaults require operator.admin for internal gateway callers; skipped persistence.";
|
||||
|
||||
export function canPersistInternalExecDirective(params: {
|
||||
surface?: string;
|
||||
gatewayClientScopes?: string[];
|
||||
}): boolean {
|
||||
if (!isInternalMessageChannel(params.surface)) {
|
||||
return true;
|
||||
}
|
||||
const scopes = params.gatewayClientScopes ?? [];
|
||||
return scopes.includes("operator.admin");
|
||||
}
|
||||
|
||||
export const formatElevatedEvent = (level: ElevatedLevel) => {
|
||||
if (level === "full") {
|
||||
return "Elevated FULL — exec runs on host with auto-approval.";
|
||||
|
|
|
|||
|
|
@ -213,6 +213,7 @@ export async function applyInlineDirectiveOverrides(params: {
|
|||
currentReasoningLevel,
|
||||
currentElevatedLevel,
|
||||
surface: ctx.Surface,
|
||||
gatewayClientScopes: ctx.GatewayClientScopes,
|
||||
});
|
||||
let statusReply: ReplyPayload | undefined;
|
||||
if (directives.hasStatusDirective && allowTextCommands && command.isAuthorizedSender) {
|
||||
|
|
@ -317,6 +318,8 @@ export async function applyInlineDirectiveOverrides(params: {
|
|||
initialModelLabel,
|
||||
formatModelSwitchEvent,
|
||||
agentCfg,
|
||||
surface: ctx.Surface,
|
||||
gatewayClientScopes: ctx.GatewayClientScopes,
|
||||
});
|
||||
provider = persisted.provider;
|
||||
model = persisted.model;
|
||||
|
|
|
|||
Loading…
Reference in New Issue