Skip to content

Commit 8ca0822

Browse files
pgondhi987pgondhi987
committed
clarify directive persistence policy
1 parent 6ac7820 commit 8ca0822

6 files changed

Lines changed: 49 additions & 33 deletions

File tree

‎docs/tools/exec.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,10 @@ Example:
164164
## Authorization model
165165

166166
`/exec` is only honored for **authorized senders** (channel allowlists/pairing plus `commands.useAccessGroups`).
167-
It updates **session state only** and does not write config. To hard-disable exec, deny it via tool
168-
policy (`tools.deny: ["exec"]` or per-agent). Host approvals still apply unless you explicitly set
169-
`security=full` and `ask=off`.
167+
It updates **session state only** and does not write config. Authorized external channel senders may
168+
set these session defaults. Internal gateway/webchat clients need `operator.admin` to persist them.
169+
To hard-disable exec, deny it via tool policy (`tools.deny: ["exec"]` or per-agent). Host approvals
170+
still apply unless you explicitly set `security=full` and `ask=off`.
170171

171172
## Exec approvals (companion app / node host)
172173

‎src/auto-reply/reply/directive-handling.impl.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ import { maybeHandleModelDirectiveInfo } from "./directive-handling.model.js";
2020
import type { HandleDirectiveOnlyParams } from "./directive-handling.params.js";
2121
import { maybeHandleQueueDirective } from "./directive-handling.queue-validation.js";
2222
import {
23-
canPersistInternalExecDirective,
24-
canPersistInternalVerboseDirective,
23+
canPersistSessionDirectiveDefaults,
2524
formatDirectiveAck,
2625
formatElevatedRuntimeHint,
2726
formatElevatedUnavailableText,
@@ -82,14 +81,14 @@ export async function handleDirectiveOnly(
8281
}),
8382
}).sandboxed;
8483
const shouldHintDirectRuntime = directives.hasElevatedDirective && !runtimeIsSandboxed;
85-
const allowInternalExecPersistence = canPersistInternalExecDirective({
84+
const allowInternalExecPersistence = canPersistSessionDirectiveDefaults({
8685
messageProvider: params.messageProvider,
8786
surface: params.surface,
8887
gatewayClientScopes: params.gatewayClientScopes,
8988
commandAuthorized: params.commandAuthorized,
9089
senderIsOwner: params.senderIsOwner,
9190
});
92-
const allowInternalVerbosePersistence = canPersistInternalVerboseDirective({
91+
const allowInternalVerbosePersistence = canPersistSessionDirectiveDefaults({
9392
messageProvider: params.messageProvider,
9493
surface: params.surface,
9594
gatewayClientScopes: params.gatewayClientScopes,

‎src/auto-reply/reply/directive-handling.mixed-inline.test.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ describe("mixed inline directives", () => {
196196
expect(sessionEntry.reasoningLevel).toBe("off");
197197
});
198198

199-
it("uses provider metadata when acknowledging mixed exec persistence denial", async () => {
199+
it("persists mixed exec defaults for authorized external senders with empty gateway scopes", async () => {
200200
const directives = parseInlineDirectives(
201201
"please reply\n/exec host=node security=allowlist ask=always node=worker-1",
202202
);
@@ -240,8 +240,8 @@ describe("mixed inline directives", () => {
240240
},
241241
});
242242

243-
expect(fastLane.directiveAck?.text).toContain("operator.admin");
244-
expect(fastLane.directiveAck?.text).not.toContain("Exec defaults set");
243+
expect(fastLane.directiveAck?.text).toContain("Exec defaults set");
244+
expect(fastLane.directiveAck?.text).not.toContain("operator.admin");
245245

246246
await persistInlineDirectives({
247247
directives,
@@ -263,12 +263,13 @@ describe("mixed inline directives", () => {
263263
agentCfg: cfg.agents?.defaults,
264264
messageProvider: "telegram",
265265
gatewayClientScopes: [],
266+
commandAuthorized: true,
266267
});
267268

268-
expect(sessionEntry.execHost).toBeUndefined();
269-
expect(sessionEntry.execSecurity).toBeUndefined();
270-
expect(sessionEntry.execAsk).toBeUndefined();
271-
expect(sessionEntry.execNode).toBeUndefined();
269+
expect(sessionEntry.execHost).toBe("node");
270+
expect(sessionEntry.execSecurity).toBe("allowlist");
271+
expect(sessionEntry.execAsk).toBe("always");
272+
expect(sessionEntry.execNode).toBe("worker-1");
272273
});
273274

274275
it("does not persist trace directives for unauthorized mixed messages", async () => {

‎src/auto-reply/reply/directive-handling.model.test.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1838,7 +1838,7 @@ describe("handleDirectiveOnly model persist behavior (fixes #1435)", () => {
18381838
});
18391839
});
18401840

1841-
describe("persistInlineDirectives internal exec scope gate", () => {
1841+
describe("persistInlineDirectives session directive persistence policy", () => {
18421842
it("skips exec persistence for internal operator.write callers", async () => {
18431843
const sessionEntry = await persistInternalOperatorWriteDirective(
18441844
"/exec host=node security=allowlist ask=always node=worker-1",
@@ -1856,7 +1856,7 @@ describe("persistInlineDirectives internal exec scope gate", () => {
18561856
expect(sessionEntry.verboseLevel).toBeUndefined();
18571857
});
18581858

1859-
it("skips exec persistence for non-webchat gateway callers without operator.admin", async () => {
1859+
it("skips exec persistence for unauthorized external callers even when gateway scopes are empty", async () => {
18601860
const sessionEntry = await persistInternalOperatorWriteDirective(
18611861
"/exec host=node security=allowlist ask=always node=worker-1",
18621862
{
@@ -1872,7 +1872,7 @@ describe("persistInlineDirectives internal exec scope gate", () => {
18721872
expect(sessionEntry.execNode).toBeUndefined();
18731873
});
18741874

1875-
it("skips verbose persistence for non-webchat gateway callers without operator.admin", async () => {
1875+
it("skips verbose persistence for unauthorized external callers even when gateway scopes are empty", async () => {
18761876
const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", {
18771877
messageProvider: "telegram",
18781878
surface: "telegram",
@@ -1882,13 +1882,14 @@ describe("persistInlineDirectives internal exec scope gate", () => {
18821882
expect(sessionEntry.verboseLevel).toBeUndefined();
18831883
});
18841884

1885-
it("allows non-webchat gateway callers with operator.admin to persist exec defaults", async () => {
1885+
it("allows authorized external callers with empty gateway scopes to persist exec defaults", async () => {
18861886
const sessionEntry = await persistInternalOperatorWriteDirective(
18871887
"/exec host=node security=allowlist ask=always node=worker-1",
18881888
{
18891889
messageProvider: "telegram",
18901890
surface: "telegram",
1891-
gatewayClientScopes: ["operator.admin"],
1891+
gatewayClientScopes: [],
1892+
commandAuthorized: true,
18921893
},
18931894
);
18941895

@@ -1898,6 +1899,17 @@ describe("persistInlineDirectives internal exec scope gate", () => {
18981899
expect(sessionEntry.execNode).toBe("worker-1");
18991900
});
19001901

1902+
it("allows authorized external callers with empty gateway scopes to persist verbose defaults", async () => {
1903+
const sessionEntry = await persistInternalOperatorWriteDirective("/verbose full", {
1904+
messageProvider: "telegram",
1905+
surface: "telegram",
1906+
gatewayClientScopes: [],
1907+
commandAuthorized: true,
1908+
});
1909+
1910+
expect(sessionEntry.verboseLevel).toBe("full");
1911+
});
1912+
19011913
it("skips exec persistence for non-webchat channel callers without gateway scopes", async () => {
19021914
const sessionEntry = await persistInternalOperatorWriteDirective(
19031915
"/exec host=node security=allowlist ask=always node=worker-1",

‎src/auto-reply/reply/directive-handling.persist.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ import { isThinkingLevelSupported, resolveSupportedThinkingLevel } from "../thin
1919
import { resolveModelSelectionFromDirective } from "./directive-handling.model-selection.js";
2020
import type { InlineDirectives } from "./directive-handling.parse.js";
2121
import {
22-
canPersistInternalExecDirective,
23-
canPersistInternalVerboseDirective,
22+
canPersistSessionDirectiveDefaults,
2423
enqueueModeSwitchEvents,
2524
} from "./directive-handling.shared.js";
2625
import type { ElevatedLevel, ReasoningLevel, ThinkLevel } from "./directives.js";
@@ -122,14 +121,14 @@ export async function persistInlineDirectives(params: {
122121
} = params;
123122
let { provider, model } = params;
124123
let thinkingRemap: PersistedThinkingLevelRemap | undefined;
125-
const allowInternalExecPersistence = canPersistInternalExecDirective({
124+
const allowInternalExecPersistence = canPersistSessionDirectiveDefaults({
126125
messageProvider: params.messageProvider,
127126
surface: params.surface,
128127
gatewayClientScopes: params.gatewayClientScopes,
129128
commandAuthorized: params.commandAuthorized,
130129
senderIsOwner: params.senderIsOwner,
131130
});
132-
const allowInternalVerbosePersistence = canPersistInternalVerboseDirective({
131+
const allowInternalVerbosePersistence = canPersistSessionDirectiveDefaults({
133132
messageProvider: params.messageProvider,
134133
surface: params.surface,
135134
gatewayClientScopes: params.gatewayClientScopes,

‎src/auto-reply/reply/directive-handling.shared.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,28 @@ export const formatInternalVerbosePersistenceDeniedText = () =>
2323
export const formatInternalVerboseCurrentReplyOnlyText = () =>
2424
"Verbose logging set for the current reply only.";
2525

26-
function canPersistInternalDirective(params: {
26+
export function canPersistSessionDirectiveDefaults(params: {
2727
messageProvider?: string;
2828
surface?: string;
2929
gatewayClientScopes?: string[];
3030
commandAuthorized?: boolean;
3131
senderIsOwner?: boolean;
3232
}): boolean {
33-
if (params.gatewayClientScopes === undefined) {
34-
const hasChannelContext =
35-
normalizeOptionalString(params.messageProvider) !== undefined ||
36-
normalizeOptionalString(params.surface) !== undefined;
37-
return !hasChannelContext || params.commandAuthorized === true || params.senderIsOwner === true;
33+
const messageProvider = normalizeOptionalString(params.messageProvider);
34+
const surface = normalizeOptionalString(params.surface);
35+
const hasChannelContext = messageProvider !== undefined || surface !== undefined;
36+
const isInternalGatewayCaller = messageProvider === "webchat" || surface === "webchat";
37+
38+
if (isInternalGatewayCaller) {
39+
return params.gatewayClientScopes?.includes("operator.admin") === true;
3840
}
39-
return params.gatewayClientScopes.includes("operator.admin");
40-
}
4141

42-
export const canPersistInternalExecDirective = canPersistInternalDirective;
43-
export const canPersistInternalVerboseDirective = canPersistInternalDirective;
42+
if (hasChannelContext) {
43+
return params.commandAuthorized === true || params.senderIsOwner === true;
44+
}
45+
46+
return true;
47+
}
4448

4549
const formatElevatedEvent = (level: ElevatedLevel) => {
4650
if (level === "full") {

0 commit comments

Comments
 (0)