From 80bc4d296f2159303a852241d8b2b56b5ee1d719 Mon Sep 17 00:00:00 2001 From: Roei K Date: Sun, 26 Jul 2026 13:11:39 +0300 Subject: [PATCH 1/4] fix(webhook): resolve a canonical turn scope for webhook agent actions A webhook agent action dispatches straight to the executor, bypassing the orchestrator inbound pipeline that resolves turn identity for real channels. The executor requires a canonical turn scope and fail-closes without one, so every webhook-triggered turn aborted at step:"context-authority" before the agent ran. Resolve the turn identity in the webhook handler, as the gateway, cron, heartbeat and notification turn initiators already do, and thread its turn scope into the request context. The resolved identity is the single authority for the turn: its display session key and turn scope describe the same conversation, so the session view and the memory/context authority cannot diverge. The principal is bound to the operator-configured mapping (hashed for delimiter safety) and the rendered session key is JSON-encoded into the conversation identity, so a crafted payload can neither forge nor widen a conversation's authority even though the template interpolates request data. Per-subject conversation isolation is preserved. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/webhooks.mdx | 11 ++ .../src/wiring/setup-gateway-routes.test.ts | 123 ++++++++++++++---- .../daemon/src/wiring/setup-gateway-routes.ts | 40 +++++- .../gateway-session-principal.test.ts | 74 ++++++++++- .../gateway-session-principal.ts | 32 +++++ 5 files changed, 247 insertions(+), 33 deletions(-) diff --git a/docs/reference/webhooks.mdx b/docs/reference/webhooks.mdx index 2bfe59a06..02f3a2f9d 100644 --- a/docs/reference/webhooks.mdx +++ b/docs/reference/webhooks.mdx @@ -174,6 +174,17 @@ hold the caller's connection open for the whole turn and, at the caller's reques timeout, trigger a re-delivery storm. Turn failures are logged and emitted as diagnostics by the daemon, not returned as this request's status. +The rendered `sessionKey` identifies the **conversation**, so repeat deliveries for +one subject continue the same conversation while different subjects stay isolated. +The turn's authority, however, is bound to the **mapping**, not to the payload: the +conversation's principal is derived from the mapping id, and the rendered key is +encoded into the conversation identity rather than concatenated into it. A crafted +payload therefore cannot forge or widen a conversation's authority even when the +template interpolates request data, and the session key you see in logs and +`comis sessions` is the fully-qualified resolved one (the rendered key appears +inside it). Sessions opened by a webhook carry a `webhook-`-prefixed principal, +keeping them distinguishable from gateway-opened ones. + **`"wake"`:** Triggers a daemon heartbeat. The `wakeMode` controls timing: - `"now"` (default): Fire the heartbeat immediately - `"next-heartbeat"`: Wait for the next scheduled heartbeat cycle diff --git a/packages/daemon/src/wiring/setup-gateway-routes.test.ts b/packages/daemon/src/wiring/setup-gateway-routes.test.ts index 3814c5d84..153a31e0f 100644 --- a/packages/daemon/src/wiring/setup-gateway-routes.test.ts +++ b/packages/daemon/src/wiring/setup-gateway-routes.test.ts @@ -48,6 +48,7 @@ import { } from "@comis/gateway"; import { createConversationLocator, + createConversationRef, formatSessionKey, generateStrongToken, runWithContext, @@ -203,35 +204,108 @@ describe("mountGatewayRoutes", () => { expect(observedContexts[0]).not.toBe(ambientContext); expect(observedContexts[1]).toBe(observedContexts[0]); expect(observedContexts[2]).toBe(observedContexts[0]); + // The turn's identity is the RESOLVED webhook identity, not a hand-built key: the + // principal is the hashed mapping (payload data can never forge it) and the + // rendered session key stays embedded in the conversation, so per-subject + // isolation and log greppability both hold. expect(observedContexts[0]).toMatchObject({ tenantId: "test", - userId: "webhook", - sessionKey: formatSessionKey({ - tenantId: "test", - agentId: "agent-b", - userId: "webhook", - channelId: "hook-session", - }), agentId: "agent-b", channelType: "webhook", trustLevel: "guest", deliveryOrigin: { tenantId: "test", - userId: "webhook", channelType: "webhook", channelId: "hook-session", }, }); + expect(observedContexts[0]?.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + const resolvedSessionKey = observedContexts[0]?.sessionKey as string; + expect(resolvedSessionKey).toContain("hook-session"); + expect(resolvedSessionKey.startsWith("test:agent:agent-b:")).toBe(true); expect(observedContexts[0]?.traceId).not.toBe(ambientContext.traceId); + // The reap probe must address the SAME conversation the turn ran in — a + // divergence here would reap the wrong (or no) drive. expect(reapNeverTaskedDrives).toHaveBeenCalledWith("agent-b", { agentId: "agent-b", - sessionKey: formatSessionKey({ - tenantId: "test", - agentId: "agent-b", - userId: "webhook", - channelId: "hook-session", - }), + sessionKey: resolvedSessionKey, + }); + }); + + it("resolves a canonical turn scope for the webhook turn (the executor's context-authority precondition)", async () => { + // The executor fail-closes when the ambient context carries no canonical turn + // scope: pi-executor requires BOTH `turnScope !== undefined` AND a constructible + // conversation ref, else it aborts with step:"context-authority" / + // errorKind:"precondition" and never runs the turn. The webhook action builds its + // own context and dispatches straight to the executor (it bypasses the orchestrator + // inbound pipeline that resolves identity for real channels), so it must resolve the + // turn scope itself — exactly as the gateway/cron/heartbeat turn initiators do. + // Pre-patch this context had no turnScope at all, so EVERY webhook-triggered agent + // action aborted before reaching the drive. + const observedContexts: Array> = []; + const execute = vi.fn(async () => { + observedContexts.push(tryGetContext()); + return { finishReason: "stop" }; + }); + const deps = createMockDeps({ + webhooksConfig: { + enabled: true, + mappings: [{ id: "m1", name: "test", agentId: "agent-b" }], + } as any, + getExecutor: vi.fn(() => ({ execute })) as any, + }); + mountGatewayRoutes(deps); + const config = vi.mocked(createMappedWebhookEndpoint).mock.calls.at(-1)![0] as any; + + await config.onAgentAction( + { id: "m1", name: "test", agentId: "agent-b" }, + "perform task", + "azdo:12816", + ); + + expect(execute).toHaveBeenCalledTimes(1); + const turnScope = observedContexts[0]?.turnScope; + expect(turnScope, "the webhook turn must carry a canonical turn scope").toBeDefined(); + if (turnScope === undefined) return; + // The precondition is the ref being CONSTRUCTIBLE, not merely the scope being present. + const ref = createConversationRef(turnScope.conversation); + expect(ref.ok, "the turn scope must yield a constructible conversation ref").toBe(true); + // Per-mapping conversation isolation: the rendered session key identifies the + // conversation, so two work items never share one partition. + expect(turnScope.endpoint.conversationId).toContain("azdo:12816"); + expect(turnScope.conversation.agentId).toBe("agent-b"); + }); + + it("isolates webhook conversations per rendered session key", async () => { + // Two different rendered session keys (two work items) must resolve to two + // DISTINCT conversations — otherwise concurrent drives would collide in one + // partition and read each other's history. + const observedContexts: Array> = []; + const execute = vi.fn(async () => { + observedContexts.push(tryGetContext()); + return { finishReason: "stop" }; }); + const deps = createMockDeps({ + webhooksConfig: { + enabled: true, + mappings: [{ id: "m1", name: "test", agentId: "agent-b" }], + } as any, + getExecutor: vi.fn(() => ({ execute })) as any, + }); + mountGatewayRoutes(deps); + const config = vi.mocked(createMappedWebhookEndpoint).mock.calls.at(-1)![0] as any; + const mapping = { id: "m1", name: "test", agentId: "agent-b" }; + + await config.onAgentAction(mapping, "task a", "azdo:12816"); + await config.onAgentAction(mapping, "task b", "azdo:12817"); + + expect(observedContexts).toHaveLength(2); + const first = observedContexts[0]?.turnScope; + const second = observedContexts[1]?.turnScope; + expect(first).toBeDefined(); + expect(second).toBeDefined(); + if (first === undefined || second === undefined) return; + expect(first.endpoint.conversationId).not.toBe(second.endpoint.conversationId); }); it("frames rendered webhook content with the resolved session delimiter before execution", async () => { @@ -286,24 +360,19 @@ describe("mountGatewayRoutes", () => { expect(replay?.delimiter).toBe(first?.delimiter); expect(observed[0]!.context).toMatchObject({ tenantId: "test", - userId: "webhook", - sessionKey: formatSessionKey({ - tenantId: "test", - agentId: "agent-b", - userId: "webhook", - channelId: "hook-session", - }), agentId: "agent-b", channelType: "webhook", trustLevel: "guest", }); + expect(observed[0]!.context?.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + expect(observed[0]!.context?.sessionKey as string).toContain("hook-session"); expect(observed[0]!.message).toMatchObject({ channelId: "hook-session", channelType: "webhook", - senderId: "webhook", attachments: [], metadata: { webhookMappingId: "m1" }, }); + expect(observed[0]!.message.senderId).toMatch(/^webhook-[a-f0-9]{64}$/); }); it("records a terminal webhook executor error as a failed action", async () => { @@ -453,12 +522,10 @@ describe("mountGatewayRoutes", () => { await cfg.onAgentAction({ id: "m1", name: "devtask" }, "build is_prime", "hook:devtask:x"); expect(reapNeverTaskedDrives).toHaveBeenCalledWith("default", { agentId: "default", - sessionKey: formatSessionKey({ - tenantId: "test", - agentId: "default", - userId: "webhook", - channelId: "hook:devtask:x", - }), + // The reap targets the resolved conversation; a template-rendered key + // containing delimiters stays intact inside it (JSON-encoded, so ":" in + // "hook:devtask:x" can never split the conversation identity). + sessionKey: expect.stringContaining("hook:devtask:x"), }); const delivered = (deps.container.eventBus.emit as any).mock.calls.find((c: unknown[]) => c[0] === "diagnostic:webhook_delivered"); expect(delivered).toBeDefined(); diff --git a/packages/daemon/src/wiring/setup-gateway-routes.ts b/packages/daemon/src/wiring/setup-gateway-routes.ts index 8e5b619ad..da1eeef54 100644 --- a/packages/daemon/src/wiring/setup-gateway-routes.ts +++ b/packages/daemon/src/wiring/setup-gateway-routes.ts @@ -60,6 +60,7 @@ import { classifyExecutionFinishReason, } from "@comis/orchestrator"; import { bindApiExecutionCancellation } from "./api-execution-cancellation.js"; +import { resolveWebhookTurnIdentity } from "./setup-gateway/gateway-session-principal.js"; interface OpenaiApiEnv extends Env { Variables: { clientScopes: readonly string[] }; @@ -316,12 +317,42 @@ export function mountGatewayRoutes(deps: GatewayRouteDeps): void { onAgentAction: async (_mapping, renderedMessage, renderedSessionKey) => { const execAgentId = _mapping.agentId ?? defaultAgentId; const routeChannelId = renderedSessionKey || "webhook"; - const sk: SessionKey = { + // Resolve the canonical turn scope BEFORE building the context. A webhook + // action dispatches straight to the executor, so nothing upstream resolves + // identity for it — and the executor fail-closes on a turn with no + // conversation authority. The resolved identity is the SINGLE authority for + // this turn: its display session key and its turn scope describe the same + // conversation, so the session view and the memory/context authority can + // never diverge. + const identity = resolveWebhookTurnIdentity({ tenantId: container.config.tenantId, agentId: execAgentId, - userId: "webhook", - channelId: routeChannelId, - }; + ...(_mapping.id === undefined ? {} : { mappingId: _mapping.id }), + renderedSessionKey: routeChannelId, + }); + if (!identity.ok) { + gatewayLogger.error( + { + webhookId: _mapping.id ?? "unknown", + agentId: execAgentId, + hint: "Verify the webhook mapping's agentId and sessionKey template resolve to a valid conversation before re-firing", + errorKind: "validation" as const, + }, + "Webhook agent action rejected because its turn identity could not be resolved", + ); + emitObservationalEventSafely({ eventBus: container.eventBus, logger: gatewayLogger }, "diagnostic:webhook_delivered", { + webhookId: _mapping.id ?? "unknown", + source: _mapping.name ?? "webhook", + event: "agent_action", + statusCode: 500, + success: false, + failureReason: "handler_error", + durationMs: 0, + timestamp: systemNowMs(), + }); + throw identity.error; + } + const sk: SessionKey = identity.value.displaySessionKey; const deliveryOrigin = createDeliveryOrigin({ tenantId: sk.tenantId, userId: sk.userId, @@ -346,6 +377,7 @@ export function mountGatewayRoutes(deps: GatewayRouteDeps): void { agentId: execAgentId, trustLevel: "guest", deliveryOrigin, + turnScope: identity.value.turnScope, }); if (!resolvedContext.ok) throw resolvedContext.error; const msg: NormalizedMessage = { diff --git a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts index bc6c1ab79..8fcdf29e6 100644 --- a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts +++ b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts @@ -1,6 +1,78 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; -import { resolveGatewayTurnIdentity } from "./gateway-session-principal.js"; +import { + resolveGatewayTurnIdentity, + resolveWebhookTurnIdentity, +} from "./gateway-session-principal.js"; + +describe("webhook session principal binding", () => { + it("isolates conversations per rendered session key while binding the principal to the mapping", () => { + // Per-subject isolation: two rendered keys (two work items) → two conversations, + // so concurrent drives never share a partition or read each other's history. + const first = resolveWebhookTurnIdentity({ + tenantId: "tenant-a", + agentId: "agent-a", + mappingId: "azdo-poll", + renderedSessionKey: "azdo:12816", + }); + const second = resolveWebhookTurnIdentity({ + tenantId: "tenant-a", + agentId: "agent-a", + mappingId: "azdo-poll", + renderedSessionKey: "azdo:12817", + }); + + expect(first.ok).toBe(true); + expect(second.ok).toBe(true); + if (!first.ok || !second.ok) return; + expect(first.value.turnScope.endpoint.conversationId).not.toBe( + second.value.turnScope.endpoint.conversationId, + ); + // Same mapping ⇒ same principal: the authority is the operator's config, not the subject. + expect(first.value.displaySessionKey.userId).toBe(second.value.displaySessionKey.userId); + expect(first.value.displaySessionKey.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + expect(first.value.displaySessionKey.agentId).toBe("agent-a"); + }); + + it("keeps a delimiter-bearing rendered key intact inside the conversation identity", () => { + // A sessionKey template renders PAYLOAD data, which can contain the ":" session-key + // delimiter. JSON-encoding it means a crafted payload cannot split the identity and + // impersonate another conversation. + const resolved = resolveWebhookTurnIdentity({ + tenantId: "tenant-a", + agentId: "agent-a", + mappingId: "m1", + renderedSessionKey: 'azdo:1:peer:forged"]', + }); + + expect(resolved.ok).toBe(true); + if (!resolved.ok) return; + expect(resolved.value.turnScope.endpoint.conversationId).toBe( + JSON.stringify(['azdo:1:peer:forged"]']), + ); + expect(resolved.value.displaySessionKey.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + }); + + it("separates mappings from each other and from the gateway principal namespace", () => { + const a = resolveWebhookTurnIdentity({ + tenantId: "t", agentId: "a", mappingId: "m1", renderedSessionKey: "s", + }); + const b = resolveWebhookTurnIdentity({ + tenantId: "t", agentId: "a", mappingId: "m2", renderedSessionKey: "s", + }); + const unmapped = resolveWebhookTurnIdentity({ + tenantId: "t", agentId: "a", renderedSessionKey: "s", + }); + + expect(a.ok && b.ok && unmapped.ok).toBe(true); + if (!a.ok || !b.ok || !unmapped.ok) return; + expect(a.value.displaySessionKey.userId).not.toBe(b.value.displaySessionKey.userId); + expect(unmapped.value.displaySessionKey.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + // The "webhook-" prefix keeps webhook principals disjoint from "gateway-" ones, + // so an operator reading a session key always knows which surface opened it. + expect(a.value.displaySessionKey.userId.startsWith("webhook-")).toBe(true); + }); +}); describe("gateway session principal binding", () => { it("isolates authenticated clients with delimiter-safe canonical principals", () => { diff --git a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts index 796f141f7..53a287dca 100644 --- a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts +++ b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts @@ -7,6 +7,38 @@ import { } from "@comis/orchestrator"; import type { Result } from "@comis/shared"; +/** + * Bind a webhook turn to its operator-configured mapping, never to payload-controlled + * identity. A webhook action dispatches straight to the executor (it bypasses the + * orchestrator inbound pipeline that resolves identity for real channels), so it must + * resolve its own canonical turn scope — the executor fail-closes without one. + * + * The principal is the MAPPING (operator config), hashed for delimiter safety, so a + * request body can never widen or forge the conversation's authority. The rendered + * session key becomes the conversation id via `JSON.stringify` (delimiter-safe by + * construction), preserving per-mapping/per-subject conversation isolation: two + * subjects never share one partition, and repeat events for one subject continue + * the same conversation. + */ +export function resolveWebhookTurnIdentity(input: { + tenantId: string; + agentId: string; + mappingId?: string; + renderedSessionKey: string; +}): Result { + const principalId = `webhook-${createHash("sha256") + .update(input.mappingId ?? "unmapped") + .digest("hex")}`; + return resolveInternalTurnIdentity({ + tenantId: input.tenantId, + agentId: input.agentId, + originKind: "control-plane", + instanceId: "webhook", + conversationId: JSON.stringify([input.renderedSessionKey]), + principalId, + }); +} + /** Bind a gateway conversation to the authenticated client, not caller-selected identity. */ export function resolveGatewayTurnIdentity(input: { tenantId: string; From aceedb4fd16503ad8388081ef497feb71e1d1bea Mon Sep 17 00:00:00 2001 From: Roei K Date: Sun, 26 Jul 2026 13:19:03 +0300 Subject: [PATCH 2/4] fix(obs): name the pre-execution abort in the session_not_found verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A turn that fail-closes before it records a session never reaches the session index, so `obs.explain` cannot distinguish a dead trigger from a mistyped reference. The verdict offered only "typo, expired, or older than the horizon", which sends an operator to re-check the reference while a real failure goes unexamined — the live cost when a whole webhook-driven pipeline was down and explain reported nothing but a missing ref. Name the pre-execution-abort possibility in the detail and add a next step that routes to the evidence such an abort DOES leave: the ERROR carrying its step and errorKind in the daemon log. Co-Authored-By: Claude Opus 5 (1M context) --- .../daemon/src/api/obs-handlers/obs-explain.test.ts | 12 ++++++++++++ packages/daemon/src/api/obs-handlers/obs-explain.ts | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/daemon/src/api/obs-handlers/obs-explain.test.ts b/packages/daemon/src/api/obs-handlers/obs-explain.test.ts index 5445995d8..7d399b513 100644 --- a/packages/daemon/src/api/obs-handlers/obs-explain.test.ts +++ b/packages/daemon/src/api/obs-handlers/obs-explain.test.ts @@ -640,6 +640,18 @@ describe("bindObsExplainHandlers", () => { ).toBe(true); // The empty session report is still well-formed (no leak, no crash). expect(r.failures).toEqual([]); + // The verdict must name the PRE-EXECUTION ABORT possibility, not only "typo / + // expired". A turn that fail-closes before recording a session (an unresolved + // conversation authority, a rejected identity, tool assembly) never reaches the + // session index, so a dead trigger presents IDENTICALLY to a bad reference. A + // verdict that only offers "typo or expired" sends the operator to re-check the + // reference while a real failure goes unexamined — the live cost when an entire + // webhook-driven pipeline was down yet `explain` reported only a missing ref. + expect(r.likelyRootCause?.detail).toMatch(/abort|before .*record|pre-execution/i); + expect( + r.likelyRootCause?.suggestedNextSteps.some((s) => /daemon log|errorKind|step/i.test(s)), + "a next step must route to the log evidence a pre-execution abort DOES leave", + ).toBe(true); }); it("a lossy ref that misroutes to a traceId-miss STILL seeds the 'did you mean' scan with the ORIGINAL ref (not the resolved empty key)", async () => { diff --git a/packages/daemon/src/api/obs-handlers/obs-explain.ts b/packages/daemon/src/api/obs-handlers/obs-explain.ts index 7d4ce043e..acedcef97 100644 --- a/packages/daemon/src/api/obs-handlers/obs-explain.ts +++ b/packages/daemon/src/api/obs-handlers/obs-explain.ts @@ -416,10 +416,11 @@ export async function assembleIncidentReportFromSources( } : { code: "session_not_found", - detail: `${missedRefField} did not resolve to any session in the index (today/yesterday); it may be a typo, expired, or older than the 2-day resolution horizon`, + detail: `${missedRefField} did not resolve to any session in the index (today/yesterday). Either the reference is wrong or older than the 2-day resolution horizon, OR the turn aborted BEFORE it recorded a session — a pre-execution fail-closed (e.g. unresolved conversation authority, a rejected identity, or tool assembly) never reaches the index, so a real failure can look like a missing reference`, suggestedNextSteps: [ `verify the ${missedRefField}, or query by sessionKey directly`, "confirm the session ended within the last two days (the session-index lookup window)", + `if the trigger is known to have fired, treat this as a possible pre-execution abort: grep the daemon log for this ${missedRefField} — a turn that failed before recording emits an ERROR carrying its step and errorKind but no session`, ], }; report.truncations.push({ From e44eaf30f594f9494640c9b7c4a1e6033a492742 Mon Sep 17 00:00:00 2001 From: comis-agent Date: Sun, 26 Jul 2026 16:05:34 +0300 Subject: [PATCH 3/4] test(webhook): pin the delivery origin to the endpoint its turn scope resolved A webhook turn's delivery origin and turn scope are one authority downstream. BackgroundTaskOriginSchema rejects the pair when they disagree ("delivery origin must agree with the resolved turn authority"), which gates background-task promotion and task extraction; the capability lease principal rejects it for a jailed session.spawn; and the autonomy wiring drops the durable principal. A turn built with a hand-made origin therefore starts and then fail-closes at each of those seams. Assert the observed context's origin agrees with its scope field by field AND that the pair parses through the schema the background-task manager uses, so the agreement is pinned where the wiring builds it rather than in a helper. --- .../src/wiring/setup-gateway-routes.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/packages/daemon/src/wiring/setup-gateway-routes.test.ts b/packages/daemon/src/wiring/setup-gateway-routes.test.ts index 153a31e0f..fbf3da227 100644 --- a/packages/daemon/src/wiring/setup-gateway-routes.test.ts +++ b/packages/daemon/src/wiring/setup-gateway-routes.test.ts @@ -47,6 +47,7 @@ import { createResponsesRoute, } from "@comis/gateway"; import { + BackgroundTaskOriginSchema, createConversationLocator, createConversationRef, formatSessionKey, @@ -308,6 +309,73 @@ describe("mountGatewayRoutes", () => { expect(first.endpoint.conversationId).not.toBe(second.endpoint.conversationId); }); + it("binds the webhook delivery origin to the SAME endpoint its turn scope resolved", async () => { + // The runtime treats the delivery origin and the turn scope as one authority + // and rejects the pair wherever they disagree: BackgroundTaskOriginSchema + // ("delivery origin must agree with the resolved turn authority") gates + // background-task promotion and task extraction, the capability lease + // principal gates a jailed session.spawn, and the autonomy wiring drops the + // durable principal. A webhook turn whose origin says one channel while its + // scope says another therefore starts, then fail-closes at every one of those + // seams. Cron and durable resume derive the origin FROM the resolved endpoint + // for exactly this reason; so must the webhook path. + const observedContexts: Array> = []; + const execute = vi.fn(async () => { + observedContexts.push(tryGetContext()); + return { finishReason: "stop" }; + }); + const deps = createMockDeps({ + webhooksConfig: { + enabled: true, + mappings: [{ id: "m1", name: "test", agentId: "agent-b" }], + } as any, + getExecutor: vi.fn(() => ({ execute })) as any, + }); + mountGatewayRoutes(deps); + const config = vi.mocked(createMappedWebhookEndpoint).mock.calls.at(-1)![0] as any; + + await config.onAgentAction( + { id: "m1", name: "test", agentId: "agent-b" }, + "perform task", + "azdo:12816", + ); + + const context = observedContexts[0]; + const turnScope = context?.turnScope; + const deliveryOrigin = context?.deliveryOrigin; + expect(turnScope).toBeDefined(); + expect(deliveryOrigin).toBeDefined(); + if (turnScope === undefined || deliveryOrigin === undefined) return; + expect(deliveryOrigin.channelType).toBe(turnScope.endpoint.channelType); + expect(deliveryOrigin.channelId).toBe(turnScope.endpoint.conversationId); + expect(deliveryOrigin.userId).toBe(turnScope.principal.principalId); + expect(deliveryOrigin.threadId).toBe(turnScope.endpoint.threadId); + // The pair must satisfy the schema the background-task manager parses with, + // not merely look consistent field by field. + const conversationRef = createConversationRef(turnScope.conversation); + expect(conversationRef.ok).toBe(true); + if (!conversationRef.ok) return; + const origin = BackgroundTaskOriginSchema.safeParse({ + turnScope, + conversationRef: conversationRef.value, + deliveryOrigin, + traceId: context?.traceId ?? null, + responseLocalePolicy: { source: "unset", enforceLocale: false }, + backgroundHopCount: 0, + }); + expect( + origin.success, + `a webhook turn must be able to promote a background task: ${JSON.stringify( + origin.success ? [] : origin.error.issues.map((issue) => issue.message), + )}`, + ).toBe(true); + // The webhook surface stays the channel of record end to end — an operator + // reading the scope, the origin or the session key sees "webhook", not an + // internal origin kind standing in for it. + expect(turnScope.endpoint.channelType).toBe("webhook"); + expect(turnScope.endpoint.conversationId).toBe("azdo:12816"); + }); + it("frames rendered webhook content with the resolved session delimiter before execution", async () => { const observed: Array<{ message: NormalizedMessage; From 4ca87af2cf53957f002e4f5c1e784fdc1095e181 Mon Sep 17 00:00:00 2001 From: comis-agent Date: Sun, 26 Jul 2026 16:05:42 +0300 Subject: [PATCH 4/4] fix(webhook): keep the webhook surface as the channel of record for its turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolved turn identity described the conversation on a different channel than the delivery origin built beside it: the endpoint carried the control-plane origin kind and a JSON-wrapped conversation id, while the origin carried "webhook" and the rendered key. Every consumer that pairs the two rejects that turn — background task promotion and task extraction (BackgroundTaskOriginSchema), the capability lease principal for a jailed session.spawn, and the durable principal in the autonomy wiring — so a webhook turn cleared the executor precondition and then fail-closed at each of those seams. Name the webhook surface as its own origin kind and use the rendered session key as the endpoint's conversation id, then derive the delivery origin FROM the resolved endpoint, as the cron and durable-resume initiators do. The JSON wrapper is unnecessary for authority: the conversation reference digests each field length-delimited, so payload data cannot forge a field boundary — a rendered key carrying the session-key separator resolves to its own conversation. The endpoint now reads "webhook" everywhere an operator looks, and the session key keeps the rendered key as a substring. --- docs/reference/webhooks.mdx | 17 +++++---- .../daemon/src/wiring/setup-gateway-routes.ts | 13 +++++-- .../gateway-session-principal.test.ts | 37 +++++++++++++------ .../gateway-session-principal.ts | 22 +++++++---- .../src/routing/internal-turn-identity.ts | 10 ++++- 5 files changed, 69 insertions(+), 30 deletions(-) diff --git a/docs/reference/webhooks.mdx b/docs/reference/webhooks.mdx index 02f3a2f9d..a42fe93ab 100644 --- a/docs/reference/webhooks.mdx +++ b/docs/reference/webhooks.mdx @@ -177,13 +177,16 @@ diagnostics by the daemon, not returned as this request's status. The rendered `sessionKey` identifies the **conversation**, so repeat deliveries for one subject continue the same conversation while different subjects stay isolated. The turn's authority, however, is bound to the **mapping**, not to the payload: the -conversation's principal is derived from the mapping id, and the rendered key is -encoded into the conversation identity rather than concatenated into it. A crafted -payload therefore cannot forge or widen a conversation's authority even when the -template interpolates request data, and the session key you see in logs and -`comis sessions` is the fully-qualified resolved one (the rendered key appears -inside it). Sessions opened by a webhook carry a `webhook-`-prefixed principal, -keeping them distinguishable from gateway-opened ones. +conversation's principal is derived from the mapping id, and the conversation +reference digests every identity field separately. A crafted payload therefore +cannot forge or widen a conversation's authority even when the template +interpolates request data — a rendered key carrying the `:` session-key separator +resolves to its own conversation, never to the one it imitates. + +The session key you see in logs and `comis sessions` is the fully-qualified +resolved one; the rendered key appears inside it, so filtering by subject still +works. Sessions opened by a webhook carry a `webhook-`-prefixed principal, keeping +them distinguishable from gateway-opened ones. **`"wake"`:** Triggers a daemon heartbeat. The `wakeMode` controls timing: - `"now"` (default): Fire the heartbeat immediately diff --git a/packages/daemon/src/wiring/setup-gateway-routes.ts b/packages/daemon/src/wiring/setup-gateway-routes.ts index da1eeef54..71b1785a4 100644 --- a/packages/daemon/src/wiring/setup-gateway-routes.ts +++ b/packages/daemon/src/wiring/setup-gateway-routes.ts @@ -353,11 +353,18 @@ export function mountGatewayRoutes(deps: GatewayRouteDeps): void { throw identity.error; } const sk: SessionKey = identity.value.displaySessionKey; + // Derive the delivery origin FROM the resolved endpoint, as the cron and + // durable-resume initiators do. The two are one authority downstream: + // background-task promotion, the capability lease principal and the + // durable principal all reject a turn whose origin names a different + // channel or conversation than its scope, so a hand-built origin would + // let the turn start and then fail-close at each of those seams. + const endpoint = identity.value.turnScope.endpoint; const deliveryOrigin = createDeliveryOrigin({ tenantId: sk.tenantId, - userId: sk.userId, - channelType: "webhook", - channelId: routeChannelId, + userId: identity.value.turnScope.principal.principalId, + channelType: endpoint.channelType, + channelId: endpoint.conversationId, }); return runWithContext({ traceId: randomUUID(), diff --git a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts index 8fcdf29e6..f7f93ae42 100644 --- a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts +++ b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.test.ts @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; +import { createConversationRef } from "@comis/core"; import { resolveGatewayTurnIdentity, resolveWebhookTurnIdentity, @@ -34,23 +35,37 @@ describe("webhook session principal binding", () => { expect(first.value.displaySessionKey.agentId).toBe("agent-a"); }); - it("keeps a delimiter-bearing rendered key intact inside the conversation identity", () => { - // A sessionKey template renders PAYLOAD data, which can contain the ":" session-key - // delimiter. JSON-encoding it means a crafted payload cannot split the identity and - // impersonate another conversation. - const resolved = resolveWebhookTurnIdentity({ + it("cannot be made to address another conversation by a delimiter-bearing rendered key", () => { + // A sessionKey template renders PAYLOAD data, which can carry the ":" and ":peer:" + // separators the human-readable session key uses. The AUTHORITY is the conversation + // reference, which digests every field length-delimited, so a crafted key lands in + // the endpoint verbatim and still cannot forge a field boundary: the forged variant + // resolves to its own conversation, never to the one it imitates. + const forged = resolveWebhookTurnIdentity({ tenantId: "tenant-a", agentId: "agent-a", mappingId: "m1", renderedSessionKey: 'azdo:1:peer:forged"]', }); + const plain = resolveWebhookTurnIdentity({ + tenantId: "tenant-a", + agentId: "agent-a", + mappingId: "m1", + renderedSessionKey: "azdo:1", + }); - expect(resolved.ok).toBe(true); - if (!resolved.ok) return; - expect(resolved.value.turnScope.endpoint.conversationId).toBe( - JSON.stringify(['azdo:1:peer:forged"]']), - ); - expect(resolved.value.displaySessionKey.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + expect(forged.ok).toBe(true); + expect(plain.ok).toBe(true); + if (!forged.ok || !plain.ok) return; + expect(forged.value.turnScope.endpoint.conversationId).toBe('azdo:1:peer:forged"]'); + const forgedRef = createConversationRef(forged.value.turnScope.conversation); + const plainRef = createConversationRef(plain.value.turnScope.conversation); + expect(forgedRef.ok && plainRef.ok).toBe(true); + if (!forgedRef.ok || !plainRef.ok) return; + expect(forgedRef.value).not.toBe(plainRef.value); + // The principal stays the hashed mapping, so the payload never widens authority. + expect(forged.value.displaySessionKey.userId).toMatch(/^webhook-[a-f0-9]{64}$/); + expect(forged.value.displaySessionKey.userId).toBe(plain.value.displaySessionKey.userId); }); it("separates mappings from each other and from the gateway principal namespace", () => { diff --git a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts index 53a287dca..ac4f16750 100644 --- a/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts +++ b/packages/daemon/src/wiring/setup-gateway/gateway-session-principal.ts @@ -13,12 +13,18 @@ import type { Result } from "@comis/shared"; * orchestrator inbound pipeline that resolves identity for real channels), so it must * resolve its own canonical turn scope — the executor fail-closes without one. * - * The principal is the MAPPING (operator config), hashed for delimiter safety, so a - * request body can never widen or forge the conversation's authority. The rendered - * session key becomes the conversation id via `JSON.stringify` (delimiter-safe by - * construction), preserving per-mapping/per-subject conversation isolation: two - * subjects never share one partition, and repeat events for one subject continue - * the same conversation. + * The principal is the MAPPING (operator config), hashed so one mapping is never a + * prefix of another, so a request body can never widen or forge the conversation's + * authority. The rendered session key becomes the endpoint's conversation id, which + * preserves per-mapping/per-subject isolation — two subjects never share one + * partition, repeat events for one subject continue the same conversation — and + * carries no delimiter risk: the conversation reference digests every field + * length-delimited, so payload data cannot forge a field boundary. + * + * The endpoint keeps the webhook's own channel type, so the delivery origin the + * caller derives from it agrees with this scope. Consumers that pair the two — + * background-task promotion, the capability lease principal, the durable principal — + * reject a turn whose origin and scope name different channels. */ export function resolveWebhookTurnIdentity(input: { tenantId: string; @@ -32,9 +38,9 @@ export function resolveWebhookTurnIdentity(input: { return resolveInternalTurnIdentity({ tenantId: input.tenantId, agentId: input.agentId, - originKind: "control-plane", + originKind: "webhook", instanceId: "webhook", - conversationId: JSON.stringify([input.renderedSessionKey]), + conversationId: input.renderedSessionKey, principalId, }); } diff --git a/packages/orchestrator/src/routing/internal-turn-identity.ts b/packages/orchestrator/src/routing/internal-turn-identity.ts index 7f26e0179..01441737f 100644 --- a/packages/orchestrator/src/routing/internal-turn-identity.ts +++ b/packages/orchestrator/src/routing/internal-turn-identity.ts @@ -7,7 +7,15 @@ import { import { err, ok, type Result } from "@comis/shared"; import { resolveRoutingPolicy } from "./routing-policy-resolver.js"; -export type InternalOriginKind = "scheduler" | "control-plane" | "durable-resume"; +/** + * The surface a turn that no channel adapter produced arrives on. It becomes the + * resolved endpoint's `channelType`, so every consumer that pairs a delivery + * origin with a turn scope — background-task promotion, capability leases, the + * durable principal — sees the same channel on both sides. Name the real surface + * here rather than folding one into another: `"webhook"` turns are triggered from + * outside and must not be indistinguishable from control-plane ones. + */ +export type InternalOriginKind = "scheduler" | "control-plane" | "durable-resume" | "webhook"; export interface InternalTurnIdentity { turnScope: ResolvedTurnScope;