From bb418a857eb0eeddce8121ee4702f6f45dcd30b6 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Thu, 28 May 2026 15:20:47 +0530 Subject: [PATCH] Clarify directive persistence authorization policy [AI] (#86369) * fix: require admin scope for persisted directive defaults * addressing codex review * fix: complete directive persistence scope gate * addressing review-skill * fix: preserve channel directive persistence * fix: require admin scope for directive default persistence * addressing codex review * fix: complete directive persistence scope handling * addressing codex review * fix: complete directive persistence gate * addressing review-skill * fix: complete directive persistence gate * addressing review-skill * clarify directive persistence policy * docs: add changelog entry for PR merge --- CHANGELOG.md | 1 + docs/tools/exec.md | 7 +- .../reply/directive-handling.fast-lane.ts | 2 + .../reply/directive-handling.impl.ts | 11 +- .../directive-handling.mixed-inline.test.ts | 76 +++++++++++ .../reply/directive-handling.model.test.ts | 126 +++++++++++++++++- .../reply/directive-handling.params.ts | 1 + .../reply/directive-handling.persist.ts | 12 +- .../reply/directive-handling.shared.ts | 35 ++--- .../reply/get-reply-directives-apply.ts | 2 + 10 files changed, 246 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ac0835af3e..90f4bd735db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Clarify directive persistence authorization policy [AI]. (#86369) Thanks @pgondhi987. - Agents/Codex: keep spawned agent cwd/workspace state separated, keep hook context prompt-local, release session locks on timeout abort, avoid session event queue self-wait, preserve shared app-server state across startup or helper failures, keep native hook relay alive across restarts, route workspace memory through tools, resolve Codex runtime models first, report quarantined dynamic tools, format `skills` command output, and bound compaction/steering retries. (#87218, #86875, #86123, #87399, #87375, #87383, #87400) Thanks @mbelinky, @Alix-007, @luoyanglang, @yetval, and @sjf. - Channels: thread canonical session keys into outbound hooks, preserve Matrix room-id case, keep fallback tool warnings mention-inert, retain delivered Slack final replies during late cleanup, continue iMessage polling after denied reactions, suppress duplicate native exec approvals, preserve Telegram SecretRef prompt config, suppress Discord recovered tool warnings, and block untrusted Teams service URLs. (#73706, #75670, #87366, #87451, #87334) Thanks @zeroaltitude, @lukeboyett, @xiaotian, and @eleqtrizit. - CLI/auth/doctor/providers: reject malformed numeric/timeout/subcommand-version inputs, wait for respawn child shutdown, bound Codex and GitHub Copilot OAuth/token requests, warm provider auth off the main thread, honor Codex response timeouts, bound local service startup, resolve GPT-5.5 without cached catalog, migrate legacy memory auto-provider config, rewrite non-canonical `api_key` auth profiles, and make doctor restart follow-ups actionable. (#87398, #86281, #87361) Thanks @Patrick-Erichsen, @samzong, @giodl73-repo, and @alkor2000. diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 73bfba9f485..016d6872b61 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -164,9 +164,10 @@ Example: ## Authorization model `/exec` is only honored for **authorized senders** (channel allowlists/pairing plus `commands.useAccessGroups`). -It updates **session state only** and does not write config. To hard-disable exec, deny it via tool -policy (`tools.deny: ["exec"]` or per-agent). Host approvals still apply unless you explicitly set -`security=full` and `ask=off`. +It updates **session state only** and does not write config. Authorized external channel senders may +set these session defaults. Internal gateway/webchat clients need `operator.admin` to persist them. +To hard-disable exec, deny it via tool policy (`tools.deny: ["exec"]` or per-agent). Host approvals +still apply unless you explicitly set `security=full` and `ask=off`. ## Exec approvals (companion app / node host) diff --git a/src/auto-reply/reply/directive-handling.fast-lane.ts b/src/auto-reply/reply/directive-handling.fast-lane.ts index 188c364a18e..a63c9a5360f 100644 --- a/src/auto-reply/reply/directive-handling.fast-lane.ts +++ b/src/auto-reply/reply/directive-handling.fast-lane.ts @@ -90,8 +90,10 @@ export async function applyInlineDirectivesFastLane( currentReasoningLevel, currentElevatedLevel, ctx, + messageProvider: ctx.Provider, surface: ctx.Surface, gatewayClientScopes: ctx.GatewayClientScopes, + commandAuthorized, senderIsOwner: params.senderIsOwner, workspaceDir: params.workspaceDir, }); diff --git a/src/auto-reply/reply/directive-handling.impl.ts b/src/auto-reply/reply/directive-handling.impl.ts index af41f436fba..2cddff0f28f 100644 --- a/src/auto-reply/reply/directive-handling.impl.ts +++ b/src/auto-reply/reply/directive-handling.impl.ts @@ -20,8 +20,7 @@ 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, - canPersistInternalVerboseDirective, + canPersistSessionDirectiveDefaults, formatDirectiveAck, formatElevatedRuntimeHint, formatElevatedUnavailableText, @@ -82,15 +81,19 @@ export async function handleDirectiveOnly( }), }).sandboxed; const shouldHintDirectRuntime = directives.hasElevatedDirective && !runtimeIsSandboxed; - const allowInternalExecPersistence = canPersistInternalExecDirective({ + const allowInternalExecPersistence = canPersistSessionDirectiveDefaults({ messageProvider: params.messageProvider, surface: params.surface, gatewayClientScopes: params.gatewayClientScopes, + commandAuthorized: params.commandAuthorized, + senderIsOwner: params.senderIsOwner, }); - const allowInternalVerbosePersistence = canPersistInternalVerboseDirective({ + const allowInternalVerbosePersistence = canPersistSessionDirectiveDefaults({ messageProvider: params.messageProvider, surface: params.surface, gatewayClientScopes: params.gatewayClientScopes, + commandAuthorized: params.commandAuthorized, + senderIsOwner: params.senderIsOwner, }); const modelInfo = await maybeHandleModelDirectiveInfo({ diff --git a/src/auto-reply/reply/directive-handling.mixed-inline.test.ts b/src/auto-reply/reply/directive-handling.mixed-inline.test.ts index d8e57ed616d..38000f8f640 100644 --- a/src/auto-reply/reply/directive-handling.mixed-inline.test.ts +++ b/src/auto-reply/reply/directive-handling.mixed-inline.test.ts @@ -196,6 +196,82 @@ describe("mixed inline directives", () => { expect(sessionEntry.reasoningLevel).toBe("off"); }); + it("persists mixed exec defaults for authorized external senders with empty gateway scopes", async () => { + const directives = parseInlineDirectives( + "please reply\n/exec host=node security=allowlist ask=always node=worker-1", + ); + const cfg = createConfig(); + const sessionEntry = createSessionEntry(); + const sessionStore = { "agent:main:telegram:user": sessionEntry }; + + const fastLane = await applyInlineDirectivesFastLane({ + directives, + commandAuthorized: true, + senderIsOwner: false, + ctx: { Provider: "telegram", GatewayClientScopes: [] } as never, + cfg, + agentId: "main", + isGroup: false, + sessionEntry, + sessionStore, + sessionKey: "agent:main:telegram:user", + storePath: undefined, + elevatedEnabled: false, + elevatedAllowed: false, + elevatedFailures: [], + messageProviderKey: "telegram", + defaultProvider: "anthropic", + defaultModel: "claude-opus-4-6", + aliasIndex: { byAlias: new Map(), byKey: new Map() }, + allowedModelKeys: new Set(), + allowedModelCatalog: [], + resetModelOverride: false, + provider: "anthropic", + model: "claude-opus-4-6", + initialModelLabel: "anthropic/claude-opus-4-6", + formatModelSwitchEvent: (label) => label, + agentCfg: cfg.agents?.defaults, + modelState: { + resolveDefaultThinkingLevel: async () => "off", + resolveThinkingCatalog: async () => [], + allowedModelKeys: new Set(), + allowedModelCatalog: [], + resetModelOverride: false, + }, + }); + + expect(fastLane.directiveAck?.text).toContain("Exec defaults set"); + expect(fastLane.directiveAck?.text).not.toContain("operator.admin"); + + await persistInlineDirectives({ + directives, + cfg, + sessionEntry, + sessionStore, + sessionKey: "agent:main:telegram:user", + storePath: undefined, + elevatedEnabled: false, + elevatedAllowed: false, + defaultProvider: "anthropic", + defaultModel: "claude-opus-4-6", + aliasIndex: { byAlias: new Map(), byKey: new Map() }, + allowedModelKeys: new Set(), + provider: "anthropic", + model: "claude-opus-4-6", + initialModelLabel: "anthropic/claude-opus-4-6", + formatModelSwitchEvent: (label) => label, + agentCfg: cfg.agents?.defaults, + messageProvider: "telegram", + gatewayClientScopes: [], + commandAuthorized: true, + }); + + expect(sessionEntry.execHost).toBe("node"); + expect(sessionEntry.execSecurity).toBe("allowlist"); + expect(sessionEntry.execAsk).toBe("always"); + expect(sessionEntry.execNode).toBe("worker-1"); + }); + it("does not persist trace directives for unauthorized mixed messages", async () => { const directives = parseInlineDirectives("please reply\n/trace raw"); const cfg = createConfig(); diff --git a/src/auto-reply/reply/directive-handling.model.test.ts b/src/auto-reply/reply/directive-handling.model.test.ts index b49604697e9..50f86bd78fb 100644 --- a/src/auto-reply/reply/directive-handling.model.test.ts +++ b/src/auto-reply/reply/directive-handling.model.test.ts @@ -1842,7 +1842,7 @@ describe("handleDirectiveOnly model persist behavior (fixes #1435)", () => { }); }); -describe("persistInlineDirectives internal exec scope gate", () => { +describe("persistInlineDirectives session directive persistence policy", () => { it("skips exec persistence for internal operator.write callers", async () => { const sessionEntry = await persistInternalOperatorWriteDirective( "/exec host=node security=allowlist ask=always node=worker-1", @@ -1860,6 +1860,130 @@ describe("persistInlineDirectives internal exec scope gate", () => { expect(sessionEntry.verboseLevel).toBeUndefined(); }); + it("skips exec persistence for unauthorized external callers even when gateway scopes are empty", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective( + "/exec host=node security=allowlist ask=always node=worker-1", + { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: [], + }, + ); + + expect(sessionEntry.execHost).toBeUndefined(); + expect(sessionEntry.execSecurity).toBeUndefined(); + expect(sessionEntry.execAsk).toBeUndefined(); + expect(sessionEntry.execNode).toBeUndefined(); + }); + + it("skips verbose persistence for unauthorized external callers even when gateway scopes are empty", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: [], + }); + + expect(sessionEntry.verboseLevel).toBeUndefined(); + }); + + it("allows authorized external callers with empty gateway scopes to persist exec defaults", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective( + "/exec host=node security=allowlist ask=always node=worker-1", + { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: [], + commandAuthorized: true, + }, + ); + + expect(sessionEntry.execHost).toBe("node"); + expect(sessionEntry.execSecurity).toBe("allowlist"); + expect(sessionEntry.execAsk).toBe("always"); + expect(sessionEntry.execNode).toBe("worker-1"); + }); + + it("allows authorized external callers with empty gateway scopes to persist verbose defaults", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: [], + commandAuthorized: true, + }); + + expect(sessionEntry.verboseLevel).toBe("full"); + }); + + it("skips exec persistence for non-webchat channel callers without gateway scopes", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective( + "/exec host=node security=allowlist ask=always node=worker-1", + { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: undefined, + }, + ); + + expect(sessionEntry.execHost).toBeUndefined(); + expect(sessionEntry.execSecurity).toBeUndefined(); + expect(sessionEntry.execAsk).toBeUndefined(); + expect(sessionEntry.execNode).toBeUndefined(); + }); + + it("skips verbose persistence for non-webchat channel callers without gateway scopes", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: undefined, + }); + + expect(sessionEntry.verboseLevel).toBeUndefined(); + }); + + it("allows exec persistence for authorized non-webchat channel callers without gateway scopes", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective( + "/exec host=node security=allowlist ask=always node=worker-1", + { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: undefined, + commandAuthorized: true, + }, + ); + + expect(sessionEntry.execHost).toBe("node"); + expect(sessionEntry.execSecurity).toBe("allowlist"); + expect(sessionEntry.execAsk).toBe("always"); + expect(sessionEntry.execNode).toBe("worker-1"); + }); + + it("allows verbose persistence for authorized non-webchat channel callers without gateway scopes", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", { + messageProvider: "telegram", + surface: "telegram", + gatewayClientScopes: undefined, + commandAuthorized: true, + }); + + expect(sessionEntry.verboseLevel).toBe("full"); + }); + + it("allows exec persistence for local callers without channel context", async () => { + const sessionEntry = await persistInternalOperatorWriteDirective( + "/exec host=node security=allowlist ask=always node=worker-1", + { + messageProvider: undefined, + surface: undefined, + gatewayClientScopes: undefined, + }, + ); + + expect(sessionEntry.execHost).toBe("node"); + expect(sessionEntry.execSecurity).toBe("allowlist"); + expect(sessionEntry.execAsk).toBe("always"); + expect(sessionEntry.execNode).toBe("worker-1"); + }); + it("treats internal provider context as authoritative over external surface metadata", async () => { const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", { messageProvider: "webchat", diff --git a/src/auto-reply/reply/directive-handling.params.ts b/src/auto-reply/reply/directive-handling.params.ts index 99e81e3e84e..700e266be7b 100644 --- a/src/auto-reply/reply/directive-handling.params.ts +++ b/src/auto-reply/reply/directive-handling.params.ts @@ -43,6 +43,7 @@ export type HandleDirectiveOnlyParams = HandleDirectiveOnlyCoreParams & { workspaceDir?: string; surface?: string; gatewayClientScopes?: string[]; + commandAuthorized?: boolean; senderIsOwner?: boolean; }; diff --git a/src/auto-reply/reply/directive-handling.persist.ts b/src/auto-reply/reply/directive-handling.persist.ts index 38a3d4677b0..006802695c2 100644 --- a/src/auto-reply/reply/directive-handling.persist.ts +++ b/src/auto-reply/reply/directive-handling.persist.ts @@ -19,8 +19,7 @@ import { isThinkingLevelSupported, resolveSupportedThinkingLevel } from "../thin import { resolveModelSelectionFromDirective } from "./directive-handling.model-selection.js"; import type { InlineDirectives } from "./directive-handling.parse.js"; import { - canPersistInternalExecDirective, - canPersistInternalVerboseDirective, + canPersistSessionDirectiveDefaults, enqueueModeSwitchEvents, } from "./directive-handling.shared.js"; import type { ElevatedLevel, ReasoningLevel, ThinkLevel } from "./directives.js"; @@ -96,6 +95,7 @@ export async function persistInlineDirectives(params: { messageProvider?: string; surface?: string; gatewayClientScopes?: string[]; + commandAuthorized?: boolean; senderIsOwner?: boolean; markLiveSwitchPending?: boolean; thinkingCatalog?: ModelCatalogEntry[]; @@ -124,15 +124,19 @@ export async function persistInlineDirectives(params: { } = params; let { provider, model } = params; let thinkingRemap: PersistedThinkingLevelRemap | undefined; - const allowInternalExecPersistence = canPersistInternalExecDirective({ + const allowInternalExecPersistence = canPersistSessionDirectiveDefaults({ messageProvider: params.messageProvider, surface: params.surface, gatewayClientScopes: params.gatewayClientScopes, + commandAuthorized: params.commandAuthorized, + senderIsOwner: params.senderIsOwner, }); - const allowInternalVerbosePersistence = canPersistInternalVerboseDirective({ + const allowInternalVerbosePersistence = canPersistSessionDirectiveDefaults({ messageProvider: params.messageProvider, surface: params.surface, gatewayClientScopes: params.gatewayClientScopes, + commandAuthorized: params.commandAuthorized, + senderIsOwner: params.senderIsOwner, }); const thinkingCatalog = params.thinkingCatalog && params.thinkingCatalog.length > 0 diff --git a/src/auto-reply/reply/directive-handling.shared.ts b/src/auto-reply/reply/directive-handling.shared.ts index b37d7d79924..e8bf8ba4112 100644 --- a/src/auto-reply/reply/directive-handling.shared.ts +++ b/src/auto-reply/reply/directive-handling.shared.ts @@ -1,6 +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 { normalizeOptionalString } from "../../shared/string-coerce.js"; import type { ElevatedLevel, ReasoningLevel } from "./directives.js"; export const formatDirectiveAck = (text: string): string => { @@ -15,31 +15,36 @@ 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."; + "Exec defaults require operator.admin for gateway callers; skipped persistence."; export const formatInternalVerbosePersistenceDeniedText = () => - "Verbose defaults require operator.admin for internal gateway callers; skipped persistence."; + "Verbose defaults require operator.admin for gateway callers; skipped persistence."; export const formatInternalVerboseCurrentReplyOnlyText = () => "Verbose logging set for the current reply only."; -function canPersistInternalDirective(params: { +export function canPersistSessionDirectiveDefaults(params: { messageProvider?: string; surface?: string; gatewayClientScopes?: string[]; + commandAuthorized?: boolean; + senderIsOwner?: boolean; }): boolean { - const authoritativeChannel = isInternalMessageChannel(params.messageProvider) - ? params.messageProvider - : params.surface; - if (!isInternalMessageChannel(authoritativeChannel)) { - return true; - } - const scopes = params.gatewayClientScopes ?? []; - return scopes.includes("operator.admin"); -} + const messageProvider = normalizeOptionalString(params.messageProvider); + const surface = normalizeOptionalString(params.surface); + const hasChannelContext = messageProvider !== undefined || surface !== undefined; + const isInternalGatewayCaller = messageProvider === "webchat" || surface === "webchat"; -export const canPersistInternalExecDirective = canPersistInternalDirective; -export const canPersistInternalVerboseDirective = canPersistInternalDirective; + if (isInternalGatewayCaller) { + return params.gatewayClientScopes?.includes("operator.admin") === true; + } + + if (hasChannelContext) { + return params.commandAuthorized === true || params.senderIsOwner === true; + } + + return true; +} const formatElevatedEvent = (level: ElevatedLevel) => { if (level === "full") { diff --git a/src/auto-reply/reply/get-reply-directives-apply.ts b/src/auto-reply/reply/get-reply-directives-apply.ts index c84323f93ed..b6c82431ab6 100644 --- a/src/auto-reply/reply/get-reply-directives-apply.ts +++ b/src/auto-reply/reply/get-reply-directives-apply.ts @@ -249,6 +249,7 @@ export async function applyInlineDirectiveOverrides(params: { messageProvider: ctx.Provider, surface: ctx.Surface, gatewayClientScopes: ctx.GatewayClientScopes, + commandAuthorized: command.isAuthorizedSender, senderIsOwner: command.senderIsOwner, }; @@ -342,6 +343,7 @@ export async function applyInlineDirectiveOverrides(params: { messageProvider: ctx.Provider, surface: ctx.Surface, gatewayClientScopes: ctx.GatewayClientScopes, + commandAuthorized: command.isAuthorizedSender, senderIsOwner: command.senderIsOwner, workspaceDir, });