fix: require admin for keyed session model changes [AI] (#111651)

* fix: gate keyed session model creation

* fix: preserve fresh keyed session creation

* fix: cover existing keyed model adoption

* fix: keep keyed session create idempotent

* fix: resolve keyed session model aliases before gating

* fix: tighten keyed session gate comparison

* docs: clarify keyed session gate fail-closed cases

* docs: explain keyed session model error gating

* fix: clarify unscoped keyed session gate

* fix: share session model patch resolution

* fix: preserve existing auth profile no-op creates

* fix: resolve subagent defaults in keyed session gate
This commit is contained in:
Pavan Kumar Gondhi
2026-07-20 16:33:33 +05:30
committed by GitHub
parent 8bec61bab1
commit 8d5ad804a6
6 changed files with 407 additions and 21 deletions
+50
View File
@@ -319,6 +319,56 @@ describe("method scope resolution", () => {
).toEqual({ allowed: false, missingScope: "operator.admin" });
});
it("keeps keyed sessions.create model selection at write scope for handler-state checks", () => {
expect(
resolveLeastPrivilegeOperatorScopesForMethod("sessions.create", {
agentId: "main",
label: "Dashboard",
model: "openai/gpt-5.5",
}),
).toEqual(["operator.write"]);
expect(
resolveLeastPrivilegeOperatorScopesForMethod("sessions.create", {
key: "agent:main:dashboard:existing",
label: "Dashboard",
}),
).toEqual(["operator.write"]);
expect(
resolveLeastPrivilegeOperatorScopesForMethod("sessions.create", {
key: "agent:main:dashboard:existing",
model: "openai/gpt-5.5",
}),
).toEqual(["operator.write"]);
expect(
resolveLeastPrivilegeOperatorScopesForMethod("sessions.create", {
key: "agent:main:dashboard:existing",
thinkingLevel: "high",
}),
).toEqual(["operator.write"]);
for (const params of [
{ key: "agent:main:dashboard:existing", model: "openai/gpt-5.5" },
{ key: "agent:main:dashboard:existing", thinkingLevel: "high" },
{
key: "agent:main:dashboard:existing",
label: "Dashboard",
model: "openai/gpt-5.5",
thinkingLevel: "high",
},
]) {
expect(
authorizeOperatorScopesForMethod("sessions.create", ["operator.write"], params),
).toEqual({
allowed: true,
});
expect(
authorizeOperatorScopesForMethod("sessions.create", ["operator.admin"], params),
).toEqual({
allowed: true,
});
}
});
it("keeps worktree target params at write scope but execNode at admin", () => {
expect(
resolveLeastPrivilegeOperatorScopesForMethod("sessions.create", {
+4 -3
View File
@@ -110,9 +110,10 @@ function resolveSessionsCreateRequiredScopes(params: unknown): OperatorScope[] {
}
// cwd targets arbitrary host checkouts; execNode routes exec onto a paired
// node host. Both match the sessions.patch execNode admin bar.
return Object.hasOwn(params, "cwd") || Object.hasOwn(params, "execNode")
? [ADMIN_SCOPE]
: [WRITE_SCOPE];
if (Object.hasOwn(params, "cwd") || Object.hasOwn(params, "execNode")) {
return [ADMIN_SCOPE];
}
return [WRITE_SCOPE];
}
function resolveSessionActionRegisteredScopes(params: unknown): OperatorScope[] | undefined {
@@ -18,7 +18,7 @@ import { formatErrorMessage } from "../../infra/errors.js";
import { isPathInside } from "../../infra/path-guards.js";
import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js";
import { resolveUserPath } from "../../utils.js";
import { ADMIN_SCOPE } from "../operator-scopes.js";
import { ADMIN_SCOPE, authorizeOperatorScopesForRequiredScope } from "../method-scopes.js";
import {
buildDashboardSessionKey,
createGatewaySession,
@@ -321,6 +321,11 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
let runError: unknown;
let runMeta: Record<string, unknown> | undefined;
let messageSeq: number | undefined;
const clientScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
const allowExistingModelSelection = authorizeOperatorScopesForRequiredScope(
ADMIN_SCOPE,
clientScopes,
).allowed;
const created = await createGatewaySession({
cfg,
key: sessionKey,
@@ -328,6 +333,7 @@ export const sessionCreateHandlers: GatewayRequestHandlers = {
label: p.label,
...(catalogTarget ? { catalogTarget: catalogTarget.target } : { model: p.model }),
thinkingLevel: p.thinkingLevel,
allowExistingModelSelection,
parentSessionKey: p.parentSessionKey,
spawnedCwd: sessionCwd,
worktree: sessionWorktree
+170
View File
@@ -1213,6 +1213,176 @@ test("sessions.create accepts an explicit key for persistent dashboard sessions"
);
});
test("sessions.create preserves write-scoped fresh keyed model selection but gates adopted rows", async () => {
const { storePath } = await createSessionStoreDir();
agentDiscoveryMock.enabled = true;
agentDiscoveryMock.models = [
{ id: "gpt-test-a", name: "A", provider: "openai" },
{ id: "gpt-test-b", name: "B", provider: "openai" },
];
testState.agentConfig = { subagents: { model: "openai/gpt-test-a" } };
const writeClient = { connect: { scopes: ["operator.write"] } } as never;
const adminClient = { connect: { scopes: ["operator.admin"] } } as never;
const unscopedClient = { connect: {} } as never;
const freshKey = "agent:main:dashboard:fresh-model";
const existingKey = "agent:main:dashboard:existing-model";
const existingProfileKey = "agent:main:dashboard:existing-profile-model";
const existingSubagentKey = "agent:main:subagent:existing-model";
await writeSessionStore({
entries: {
[existingKey]: sessionStoreEntry("sess-existing", {
providerOverride: "openai",
modelOverride: "gpt-test-a",
thinkingLevel: "low",
}),
[existingProfileKey]: sessionStoreEntry("sess-existing-profile", {
providerOverride: "openai",
modelOverride: "gpt-test-a",
authProfileOverride: "work",
authProfileOverrideSource: "user",
}),
[existingSubagentKey]: sessionStoreEntry("sess-existing-subagent"),
},
});
const fresh = await directSessionReq<{
entry?: { providerOverride?: string; modelOverride?: string };
}>("sessions.create", { key: freshKey, model: "openai/gpt-test-a" }, { client: writeClient });
expect(fresh.ok, JSON.stringify(fresh.error)).toBe(true);
expect(fresh.payload?.entry).toMatchObject({
providerOverride: "openai",
modelOverride: "gpt-test-a",
});
const sameSelection = await directSessionReq<{
entry?: { providerOverride?: string; modelOverride?: string; thinkingLevel?: string };
}>(
"sessions.create",
{ key: existingKey, model: "openai/gpt-test-a", thinkingLevel: "low" },
{ client: writeClient },
);
expect(sameSelection.ok, JSON.stringify(sameSelection.error)).toBe(true);
expect(sameSelection.payload?.entry).toMatchObject({
providerOverride: "openai",
modelOverride: "gpt-test-a",
thinkingLevel: "low",
});
const sameSubagentSelection = await directSessionReq<{
entry?: { providerOverride?: string; modelOverride?: string };
}>(
"sessions.create",
{ key: existingSubagentKey, model: "openai/gpt-test-a" },
{ client: writeClient },
);
expect(sameSubagentSelection.ok, JSON.stringify(sameSubagentSelection.error)).toBe(true);
expect(sameSubagentSelection.payload?.entry).toMatchObject({
providerOverride: "openai",
modelOverride: "gpt-test-a",
});
const sameSelectionWithProfile = await directSessionReq<{
entry?: { providerOverride?: string; modelOverride?: string; authProfileOverride?: string };
}>(
"sessions.create",
{ key: existingProfileKey, model: "openai/gpt-test-a" },
{ client: writeClient },
);
expect(sameSelectionWithProfile.ok, JSON.stringify(sameSelectionWithProfile.error)).toBe(true);
expect(sameSelectionWithProfile.payload?.entry).toMatchObject({
providerOverride: "openai",
modelOverride: "gpt-test-a",
authProfileOverride: "work",
});
const profileDenied = await directSessionReq(
"sessions.create",
{ key: existingProfileKey, model: "openai/gpt-test-a@other" },
{ client: writeClient },
);
expect(profileDenied.ok).toBe(false);
expect(profileDenied.error).toMatchObject({
code: "FORBIDDEN",
message: "missing scope: operator.admin",
});
const denied = await directSessionReq(
"sessions.create",
{ key: existingKey, model: "openai/gpt-test-b" },
{ client: writeClient },
);
expect(denied.ok).toBe(false);
expect(denied.error).toMatchObject({
code: "FORBIDDEN",
message: "missing scope: operator.admin",
});
const unscopedDenied = await directSessionReq(
"sessions.create",
{ key: existingKey, model: "openai/gpt-test-b" },
{ client: unscopedClient },
);
expect(unscopedDenied.ok).toBe(false);
expect(unscopedDenied.error).toMatchObject({
code: "FORBIDDEN",
message: "missing scope: operator.admin",
});
testState.agentConfig = {
models: {
"openai/gpt-test-b": { alias: "gpt-test-a" },
},
};
const aliasDenied = await directSessionReq(
"sessions.create",
{ key: existingKey, model: "gpt-test-a" },
{ client: writeClient },
);
expect(aliasDenied.ok).toBe(false);
expect(aliasDenied.error).toMatchObject({
code: "FORBIDDEN",
message: "missing scope: operator.admin",
});
expect(loadSessionEntry({ sessionKey: existingKey, storePath })).toMatchObject({
sessionId: "sess-existing",
providerOverride: "openai",
modelOverride: "gpt-test-a",
thinkingLevel: "low",
});
expect(loadSessionEntry({ sessionKey: existingProfileKey, storePath })).toMatchObject({
sessionId: "sess-existing-profile",
providerOverride: "openai",
modelOverride: "gpt-test-a",
authProfileOverride: "work",
});
const thinkingDenied = await directSessionReq(
"sessions.create",
{ key: existingKey, thinkingLevel: "high" },
{ client: writeClient },
);
expect(thinkingDenied.ok).toBe(false);
expect(thinkingDenied.error).toMatchObject({
code: "FORBIDDEN",
message: "missing scope: operator.admin",
});
const admin = await directSessionReq<{
entry?: { providerOverride?: string; modelOverride?: string; thinkingLevel?: string };
}>(
"sessions.create",
{ key: existingKey, model: "openai/gpt-test-b", thinkingLevel: "high" },
{ client: adminClient },
);
expect(admin.ok, JSON.stringify(admin.error)).toBe(true);
expect(admin.payload?.entry).toMatchObject({
providerOverride: "openai",
modelOverride: "gpt-test-b",
thinkingLevel: "high",
});
});
test("sessions.create scopes the main alias to the requested agent", async () => {
const { storePath } = await createSessionStoreDir();
+134 -3
View File
@@ -7,6 +7,7 @@ import {
ErrorCodes,
type ErrorShape,
errorShape,
missingScopeErrorShape,
} from "../../packages/gateway-protocol/src/index.js";
import { normalizeOptionalAgentRuntimeId } from "../agents/agent-runtime-id.js";
import {
@@ -16,6 +17,10 @@ import {
} from "../agents/agent-scope.js";
import { isEmbeddedAgentRunActive } from "../agents/embedded-agent.js";
import type { ModelCatalogEntry } from "../agents/model-catalog.types.js";
import {
resolveDefaultModelForAgent,
resolveSubagentConfiguredModelSelection,
} from "../agents/model-selection.js";
import { resolveSessionModelRef } from "../agents/session-model-ref.js";
import {
forkSessionFromParent,
@@ -36,6 +41,7 @@ import {
triggerInternalHook,
} from "../hooks/internal-hooks.js";
import {
isSubagentSessionKey,
normalizeAgentId,
parseAgentSessionKey,
resolveAgentIdFromSessionKey,
@@ -52,10 +58,12 @@ import {
runExclusiveSessionLifecycleMutation,
} from "../sessions/session-lifecycle-admission.js";
import { createLazyRuntimeModule } from "../shared/lazy-runtime.js";
import { ADMIN_SCOPE } from "./operator-scopes.js";
import { buildForkedGatewaySessionEntry } from "./session-create-fork-entry.js";
import { shouldPreserveSessionAuthProfileOverride } from "./session-model-patch-origin.js";
import { resolveSessionStoreAgentId, resolveSessionStoreKey } from "./session-store-key.js";
import { loadSessionEntry, resolveGatewaySessionStoreTarget } from "./session-utils.js";
import { applySessionsPatchToStore } from "./sessions-patch.js";
import { applySessionsPatchToStore, resolveSessionPatchModelSelection } from "./sessions-patch.js";
type TrustedCatalogSessionTarget = {
model: string;
@@ -71,6 +79,94 @@ type RequestedSessionAgentIdResolution =
| { ok: true; agentId?: string }
| { ok: false; error: ErrorShape };
async function existingModelSelectionWouldChange(params: {
cfg: OpenClawConfig;
catalogModel?: string;
defaultModel: string;
defaultProvider: string;
existingEntry: SessionEntry;
loadGatewayModelCatalog?: () => Promise<ModelCatalogEntry[]>;
requestedModel?: string;
requestedThinkingLevel?: string;
subagentModelHint?: string;
}): Promise<boolean> {
if (params.catalogModel) {
// Public catalog creates cannot include a key, and the service rejects
// catalog targets for existing rows. If a trusted caller reaches this,
// keep catalog-owned model/runtime adoption fail-closed.
return true;
}
const requestedThinkingLevel = normalizeOptionalString(params.requestedThinkingLevel);
if (
requestedThinkingLevel &&
requestedThinkingLevel !== normalizeOptionalString(params.existingEntry.thinkingLevel)
) {
return true;
}
const requestedModel = normalizeOptionalString(params.requestedModel);
if (!requestedModel) {
return false;
}
if (!params.loadGatewayModelCatalog) {
// Public/TUI model selection paths provide the catalog loader used by the
// patch resolver. Without it, an existing-row model request cannot prove
// it is a no-op, so non-admin callers must not reach the mutation path.
return true;
}
const catalog = await params.loadGatewayModelCatalog();
const resolved = resolveSessionPatchModelSelection({
cfg: params.cfg,
catalog,
raw: requestedModel,
defaultProvider: params.defaultProvider,
defaultModel: params.defaultModel,
subagentModelHint: params.subagentModelHint,
});
if (!resolved.ok) {
// Admin callers still receive the precise model error from sessions.patch.
// Non-admin existing-row creates fail closed before that mutation path.
return true;
}
let existingProvider =
normalizeOptionalString(params.existingEntry.providerOverride) ?? params.defaultProvider;
let existingModel =
normalizeOptionalString(params.existingEntry.modelOverride) ?? params.defaultModel;
if (!normalizeOptionalString(params.existingEntry.modelOverride) && params.subagentModelHint) {
const resolvedSubagentDefault = resolveSessionPatchModelSelection({
cfg: params.cfg,
catalog,
raw: params.subagentModelHint,
defaultProvider: params.defaultProvider,
defaultModel: params.defaultModel,
});
if (!resolvedSubagentDefault.ok) {
return true;
}
if (!normalizeOptionalString(params.existingEntry.providerOverride)) {
existingProvider = resolvedSubagentDefault.provider;
}
existingModel = resolvedSubagentDefault.model;
}
const existingProfile = normalizeOptionalString(params.existingEntry.authProfileOverride);
const requestedProfile = normalizeOptionalString(resolved.profile);
const profileWouldChange =
requestedProfile !== undefined
? requestedProfile !== existingProfile
: existingProfile !== undefined &&
!shouldPreserveSessionAuthProfileOverride({
cfg: params.cfg,
currentProvider:
params.existingEntry.providerOverride ??
params.existingEntry.modelProvider ??
params.defaultProvider,
entry: params.existingEntry,
provider: resolved.provider,
});
return (
resolved.provider !== existingProvider || resolved.model !== existingModel || profileWouldChange
);
}
export function resolveRequestedSessionAgentId(
cfg: OpenClawConfig,
key: string,
@@ -187,6 +283,8 @@ export async function createGatewaySession(params: {
loadGatewayModelCatalog?: () => Promise<ModelCatalogEntry[]>;
/** Trusted in-process initializer; never populated from public Gateway params. */
initialEntry?: TrustedInitialSessionEntry;
/** Public callers need admin before reconfiguring an adopted keyed session. */
allowExistingModelSelection?: boolean;
/** Exact harness namespace authorized by the scoped plugin runtime. */
authorizedAgentHarnessId?: string;
/** Exact plugin namespace authorized by the scoped plugin runtime. */
@@ -566,6 +664,39 @@ export async function createGatewaySession(params: {
),
};
}
const requestedModel = normalizeOptionalString(params.model);
const requestedThinkingLevel = normalizeOptionalString(params.thinkingLevel);
if (existingEntry?.sessionId && params.allowExistingModelSelection !== true) {
const gateDefaultModel = resolveDefaultModelForAgent({
cfg: params.cfg,
agentId: target.agentId,
});
const modelSelectionWouldChange = await existingModelSelectionWouldChange({
cfg: params.cfg,
catalogModel,
defaultModel: gateDefaultModel.model,
defaultProvider: gateDefaultModel.provider,
existingEntry,
loadGatewayModelCatalog: params.loadGatewayModelCatalog,
requestedModel,
requestedThinkingLevel,
subagentModelHint: isSubagentSessionKey(target.canonicalKey)
? resolveSubagentConfiguredModelSelection({
cfg: params.cfg,
agentId: target.agentId,
})
: undefined,
});
if (modelSelectionWouldChange) {
return {
ok: false,
error: missingScopeErrorShape({
missingScope: ADMIN_SCOPE,
requiredScopes: [ADMIN_SCOPE],
}),
};
}
}
const patched = await applySessionsPatchToStore({
cfg: params.cfg,
store: sessionEntries,
@@ -574,8 +705,8 @@ export async function createGatewaySession(params: {
patch: {
key: target.canonicalKey,
label: normalizeOptionalString(params.label),
model: catalogModel ?? normalizeOptionalString(params.model),
thinkingLevel: normalizeOptionalString(params.thinkingLevel),
model: catalogModel ?? requestedModel,
thinkingLevel: requestedThinkingLevel,
},
loadGatewayModelCatalog: params.loadGatewayModelCatalog,
authorizedAgentHarnessId: params.authorizedAgentHarnessId,
+42 -14
View File
@@ -79,6 +79,38 @@ function invalid(message: string): { ok: false; error: ErrorShape } {
return { ok: false, error: errorShape(ErrorCodes.INVALID_REQUEST, message) };
}
export function resolveSessionPatchModelSelection(params: {
cfg: OpenClawConfig;
catalog: ModelCatalogEntry[];
raw: string;
defaultProvider: string;
defaultModel: string;
subagentModelHint?: string;
}):
| { ok: true; provider: string; model: string; profile?: string; isDefault: boolean }
| { ok: false; error: string } {
const { model: modelWithoutProfile, profile } = splitTrailingAuthProfile(params.raw);
const resolved = resolveAllowedModelRef({
cfg: params.cfg,
catalog: params.catalog,
raw: modelWithoutProfile,
defaultProvider: params.defaultProvider,
defaultModel: params.subagentModelHint ?? params.defaultModel,
});
if ("error" in resolved) {
return { ok: false, error: resolved.error };
}
return {
ok: true,
provider: resolved.ref.provider,
model: resolved.ref.model,
...(profile ? { profile } : {}),
isDefault:
resolved.ref.provider === params.defaultProvider &&
resolved.ref.model === params.defaultModel,
};
}
function normalizeExecSecurity(raw: string): "deny" | "allowlist" | "full" | undefined {
const normalized = normalizeOptionalLowercaseString(raw);
if (normalized === "deny" || normalized === "allowlist" || normalized === "full") {
@@ -680,34 +712,30 @@ export async function projectSessionsPatchEntry(params: {
error: errorShape(ErrorCodes.UNAVAILABLE, "model catalog unavailable"),
};
}
const { model: modelWithoutProfile, profile: trailingProfile } =
splitTrailingAuthProfile(trimmed);
const resolved = resolveAllowedModelRef({
const resolved = resolveSessionPatchModelSelection({
cfg,
catalog,
raw: modelWithoutProfile,
raw: trimmed,
defaultProvider: resolvedDefault.provider,
defaultModel: subagentModelHint ?? resolvedDefault.model,
defaultModel: resolvedDefault.model,
subagentModelHint,
});
if ("error" in resolved) {
if (!resolved.ok) {
return invalid(resolved.error);
}
const isDefault =
resolved.ref.provider === resolvedDefault.provider &&
resolved.ref.model === resolvedDefault.model;
applyModelOverrideToSessionEntry({
entry: next,
selection: {
provider: resolved.ref.provider,
model: resolved.ref.model,
isDefault,
provider: resolved.provider,
model: resolved.model,
isDefault: resolved.isDefault,
},
profileOverride: trailingProfile || undefined,
profileOverride: resolved.profile,
preserveAuthProfileOverride: shouldPreserveSessionAuthProfileOverride({
cfg,
currentProvider: next.providerOverride ?? next.modelProvider ?? resolvedDefault.provider,
entry: next,
provider: resolved.ref.provider,
provider: resolved.provider,
}),
markLiveSwitchPending: true,
});