From 637da87a5da10cf079224fcd102725bce1c5ee57 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 19:15:01 -0700 Subject: [PATCH] fix(gateway): tools.invoke must carry the caller's host-minted role authority (#129725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(gateway): tools.invoke must carry the caller's host-minted role authority The connect handshake resolves each connection's authority once and stores it server-side (shared-secret operator owners mint system authority there). tools.invoke discarded that fact and re-derived ownership from scopes, so a shared-secret caller with no durable profile resolved to the deny-by-default role and was refused dispatch on its own agents — while the same connection could still mutate sessions directly. Carry client.internal.operatorRoleActor into the synthetic dispatch client and keep the scope-derived fallback for callers that have no connection actor (HTTP). Regression test fails pre-fix with the FORBIDDEN agent-allowlist error. * test(opencode): close the fake CLI before exec to stop ETXTBSY flakes The catalog suite wrote the fake opencode executable and spawned it immediately. Under parallel CI shards the write handle could still be open at exec time, so the launch failed with ETXTBSY and failed the shard. Write through an explicit file handle with an fsync before close so the binary is fully durable before the first spawn. * fix(ci): repair red main type and lint gates Two gates were failing on main independently of this branch: - extensions/qa-lab cleanup tests still built OpenClawCrablineChannelDriverSelection with the retired smokeArtifactPath and a stale capabilityMatrixPath, so check:test-types failed after the readiness-artifact change (#124189). Align both fixtures with the current type and its pinned constants. - scripts/github/release-validation-campaign.d.mts declared the Actions Octokit client as any (#129726), tripping no-explicit-any. Declare the structural subset the publisher actually calls instead of suppressing the rule. Verified failing on clean origin/main before the fix. --- extensions/opencode/session-catalog.test.ts | 10 +++- .../src/suite-run-isolated.cleanup.test.ts | 4 +- ...uite-runtime-parity-runner.cleanup.test.ts | 4 +- .../github/release-validation-campaign.d.mts | 21 ++++++++- src/gateway/server-methods/tools-invoke.ts | 1 + src/gateway/tools-invoke-http.test.ts | 46 +++++++++++++++++++ src/gateway/tools-invoke-shared.ts | 12 +++-- 7 files changed, 89 insertions(+), 9 deletions(-) diff --git a/extensions/opencode/session-catalog.test.ts b/extensions/opencode/session-catalog.test.ts index a183905e81cd..36eb89bde1b2 100644 --- a/extensions/opencode/session-catalog.test.ts +++ b/extensions/opencode/session-catalog.test.ts @@ -362,7 +362,15 @@ if (args[0] === "--pure" && args[1] === "db" && args.includes("--format") && arg process.exitCode = 2; } `; - await fs.writeFile(executable, script); + // Flush and close the executable before exec: a still-open write handle makes + // the immediately following spawn fail with ETXTBSY under parallel CI shards. + const executableHandle = await fs.open(executable, "w"); + try { + await executableHandle.writeFile(script); + await executableHandle.sync(); + } finally { + await executableHandle.close(); + } if (process.platform === "win32") { await fs.writeFile(path.join(directory, "opencode.js"), script); // This exact direct-forwarder shape is parsed into a Node entrypoint; diff --git a/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts b/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts index d675d3327a22..ba67afa2b33b 100644 --- a/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts +++ b/extensions/qa-lab/src/suite-run-isolated.cleanup.test.ts @@ -242,10 +242,10 @@ describe("isolated QA suite transport cleanup", () => { it("keeps Crabline workers concurrent while publishing readiness only from the final aggregate", async () => { const lab = createCleanupTestLab(); const selection = { - capabilityMatrixPath: "crabline-fake-provider-capabilities.json", + capabilityMatrixPath: "crabline-channel-driver-capabilities.json", channel: "telegram", channelDriver: "crabline", - smokeArtifactPath: "crabline-fake-provider-smoke.json", + providerReadinessArtifactPath: "crabline-provider-readiness.json", } as const; let activeWorkers = 0; let maxActiveWorkers = 0; diff --git a/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts b/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts index b2239839c9fc..748e867f667c 100644 --- a/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts +++ b/extensions/qa-lab/src/suite-runtime-parity-runner.cleanup.test.ts @@ -284,10 +284,10 @@ describe("runtime parity suite transport cleanup", () => { mocks.writeQaSuiteArtifacts.mockClear(); const scenario = makeQaSuiteTestScenario("runtime-cleanup"); const selection = { - capabilityMatrixPath: "crabline-fake-provider-capabilities.json", + capabilityMatrixPath: "crabline-channel-driver-capabilities.json", channel: "telegram", channelDriver: "crabline", - smokeArtifactPath: "crabline-fake-provider-smoke.json", + providerReadinessArtifactPath: "crabline-provider-readiness.json", } as const; const parentLab = createCleanupTestLab(); const openClawLab = createCleanupTestLab(); diff --git a/scripts/github/release-validation-campaign.d.mts b/scripts/github/release-validation-campaign.d.mts index d087bd88720a..66436bbc789d 100644 --- a/scripts/github/release-validation-campaign.d.mts +++ b/scripts/github/release-validation-campaign.d.mts @@ -27,8 +27,27 @@ export function validateReleaseValidationCampaignArtifact( }, ): ReleaseValidationCampaignArtifact; +/** + * Structural subset of the Actions-provided Octokit client this publisher uses. + * Declared locally so the script keeps a real contract without depending on + * Octokit's generated types from a plain-Node script surface. + */ +export type ReleaseValidationCampaignGitHubClient = { + rest: { + issues: { + get(params: Record): Promise<{ data: unknown }>; + getLabel(params: Record): Promise; + createLabel(params: Record): Promise; + createComment(params: Record): Promise; + update(params: Record): Promise<{ data: unknown }>; + listForRepo: unknown; + }; + }; + paginate(route: unknown, params: Record): Promise; +}; + export function runReleaseValidationCampaignPublish(params: { - github: any; + github: ReleaseValidationCampaignGitHubClient; context: { repo: { owner: string; repo: string } }; core: { info(message: string): void; setOutput?(name: string, value: string): void }; artifact: unknown; diff --git a/src/gateway/server-methods/tools-invoke.ts b/src/gateway/server-methods/tools-invoke.ts index 21c8db921888..2071bb38ad45 100644 --- a/src/gateway/server-methods/tools-invoke.ts +++ b/src/gateway/server-methods/tools-invoke.ts @@ -55,6 +55,7 @@ export const toolsInvokeHandlers: GatewayRequestHandlers = { cfg: context.getRuntimeConfig(), input: params, authenticatedUserProfile: client?.authenticatedUserProfile, + operatorRoleActor: client?.internal?.operatorRoleActor, operatorScopes: client?.connect.scopes, senderIsOwner: client?.connect?.scopes?.includes("operator.admin"), clientCaps: client?.connect?.caps, diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 4cfb6bf15176..bd54aedcd138 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -473,6 +473,7 @@ const invokeToolsRpc = async ( hasAvatar: boolean; updatedAt: number; }, + internal?: { operatorRoleActor?: { kind: "system" } | { kind: "operator"; profileId: string } }, ) => { const respond = vi.fn(); await expectDefined( @@ -484,6 +485,7 @@ const invokeToolsRpc = async ( context: { getRuntimeConfig: () => cfg } as never, client: { ...(authenticatedUserProfile ? { authenticatedUserProfile } : {}), + ...(internal ? { internal } : {}), connect: { role: "operator", scopes, @@ -572,6 +574,50 @@ describe("POST /tools/invoke", () => { }); }); + it("preserves host-minted system authority for a shared-secret caller under roles", async () => { + await withOpenClawTestState({ label: "tools-invoke-system-authority" }, async () => { + const owner = ensureProfileForEmail("sysauth-owner@example.test"); + const sessionKey = "agent:main:sysauth-primary"; + const entry = { + sessionId: "sysauth-primary-session", + updatedAt: 1, + visibility: "shared" as const, + createdActor: { type: "human" as const, id: owner.id }, + }; + await upsertSessionEntryCore({ agentId: "main", sessionKey }, entry); + sessionEntries.set(sessionKey, entry); + cfg = { + agents: { list: [{ id: "main", default: true, tools: { allow: ["agents_list"] } }] }, + gateway: { + roles: { + default: "guest", + definitions: { + guest: { + sessions: { others: "view" }, + agents: ["guest-agent"], + scopes: ["operator.write"], + }, + }, + }, + }, + }; + + // Shared-secret operator owners have no durable profile; connect mints system + // authority on the connection. Dispatch must carry that fact forward instead of + // re-deriving ownership from scopes, or the caller is denied its own agent. + const call = await invokeToolsRpc( + { name: "agents_list", args: {}, sessionKey }, + ["operator.write"], + undefined, + undefined, + undefined, + { operatorRoleActor: { kind: "system" } }, + ); + + expect(call?.[1]).toMatchObject({ ok: true, toolName: "agents_list" }); + }); + }); + it("rejects a nested sessions_send target that the operator cannot mutate", async () => { await withOpenClawTestState({ label: "tools-invoke-foreign-session" }, async () => { const owner = ensureProfileForEmail("owner@example.test"); diff --git a/src/gateway/tools-invoke-shared.ts b/src/gateway/tools-invoke-shared.ts index d7788a05731a..ece49661d474 100644 --- a/src/gateway/tools-invoke-shared.ts +++ b/src/gateway/tools-invoke-shared.ts @@ -177,6 +177,8 @@ type InvokeGatewayToolParams = { agentTo?: string; agentThreadId?: string; authenticatedUserProfile?: GatewayClient["authenticatedUserProfile"]; + /** Host-minted authority from the calling connection; never derived from wire params. */ + operatorRoleActor?: NonNullable["operatorRoleActor"]; operatorScopes?: readonly string[]; senderIsOwner?: boolean; clientCaps?: string[]; @@ -248,11 +250,15 @@ async function invokeGatewayToolWithSignal( const authenticatedUserProfile = params.cfg.gateway?.roles ? params.authenticatedUserProfile : undefined; + // The calling connection already resolved its authority at connect (shared-secret + // owners mint system authority there). Carry that exact fact forward instead of + // re-deriving it from scopes, or role boundaries deny the caller's own dispatch. + const operatorRoleActor = + params.operatorRoleActor ?? + (params.senderIsOwner && !authenticatedUserProfile ? { kind: "system" as const } : undefined); const client = createSyntheticPluginRuntimeClient({ ...(authenticatedUserProfile ? { authenticatedUserProfile } : {}), - ...(params.senderIsOwner && !authenticatedUserProfile - ? { operatorRoleActor: { kind: "system" as const } } - : {}), + ...(operatorRoleActor ? { operatorRoleActor } : {}), scopes: params.senderIsOwner ? [ADMIN_SCOPE] : [...(params.operatorScopes ?? [])], }); const primarySessionAuthorizationError = authorizeResolvedSessionMutation({