From 0f8ed83f27ab81a2826495dbaa30aa80520380f5 Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Mon, 25 May 2026 09:03:38 +0200 Subject: [PATCH] fix(relay): harden approval action routing --- extensions/relay/adapters/discord/runtime.ts | 69 +++++++++++------ extensions/relay/adapters/slack/runtime.ts | 74 +++++++++++++------ extensions/relay/adapters/telegram/runtime.ts | 64 +++++++++------- extensions/relay/broker/process.js | 48 ++++++++++-- extensions/relay/core/approval-gates.ts | 32 +++++++- extensions/relay/runtime/extension-runtime.ts | 5 +- tests/discord-runtime.test.ts | 29 +++++++- tests/relay/approval-gates.test.ts | 11 ++- tests/runtime.test.ts | 22 ++++++ tests/slack-runtime.test.ts | 38 +++++++++- 10 files changed, 304 insertions(+), 88 deletions(-) diff --git a/extensions/relay/adapters/discord/runtime.ts b/extensions/relay/adapters/discord/runtime.ts index 130ce0c..9d3ff94 100644 --- a/extensions/relay/adapters/discord/runtime.ts +++ b/extensions/relay/adapters/discord/runtime.ts @@ -20,7 +20,7 @@ import { deliverWorkspaceFileToRequester, formatRequesterFileDeliveryResult, par import { classifySharedRoomEvent, normalizeMachineSelector, parseSharedRoomSessionsArgs, parseSharedRoomToArgs, parseSharedRoomUseArgs, resolveSharedRoomMachineTarget, sharedRoomAddressingFromEvent, sharedRoomMachineIdentity, type SharedRoomAddressing, type SharedRoomMachineIdentity } from "../../core/shared-room.js"; import { buildDelegatedTaskPrompt, delegationCommandFromAction, delegationIngressEventKey, delegationRoomFromMessage, evaluateDelegationIngress, isPeerBotIdentity } from "../../core/agent-delegation-runtime.js"; import { transitionDelegationTask, type DelegationTaskRecord } from "../../core/agent-delegation.js"; -import { parseApprovalActionData, parseApprovalTextCommand } from "../../core/approval-gates.js"; +import { parseApprovalActionData, parseApprovalTextCommand, type ApprovalDecisionKind, type ApprovalDecisionResult } from "../../core/approval-gates.js"; const DISCORD_CHANNEL = "discord" as const; const IMAGE_PROMPT_FALLBACK = "Please inspect the attached image."; @@ -255,6 +255,7 @@ export class DiscordRuntime { } return; } + if (routedCommand && await this.handleApprovalCommand(routedMessage, routedCommand)) return; const preferredSessionKey = routedCommand?.name === "to" ? await this.targetSessionKeyForToCommand(routedMessage, routedCommand.args) : await this.sharedRoomPreferredSessionKey(routedMessage); const binding = await this.findDiscordBinding(routedMessage, { preferredSessionKey }); if (!binding || !isDiscordIdentityAllowed(routedMessage.sender, this.config.discord)) { @@ -395,21 +396,56 @@ export class DiscordRuntime { private async handleApprovalAction(action: ChannelInboundAction): Promise { const parsed = parseApprovalActionData(action.actionData); if (!parsed) return false; - const binding = await this.findDiscordBinding(action); - const route = binding ? this.routes.get(binding.sessionKey) : undefined; - if (!binding || !route?.actions.resolveApprovalDecision) { - await this.adapter?.answerAction(action.actionId, { text: "Approval request is stale.", alert: true }); + if (!this.config.discord || !isDiscordIdentityAllowed(action.sender, this.config.discord)) { + await this.adapter?.answerAction(action.actionId, { text: "This Discord action is not authorized.", alert: true }); return true; } - const result = await route.actions.resolveApprovalDecision({ - approvalId: parsed.approvalId, - decision: parsed.decision, + const result = await this.resolveApprovalDecisionFromDiscord(action, parsed.approvalId, parsed.decision); + await this.adapter?.answerAction(action.actionId, { text: result.message, alert: !result.ok }); + return true; + } + + private async resolveApprovalDecisionFromDiscord( + message: Pick, + approvalId: string, + decision: ApprovalDecisionKind, + ): Promise { + const request = await this.store.getApprovalRequest(approvalId); + if (!request || request.requester.channel !== DISCORD_CHANNEL || (request.requester.instanceId ?? "default") !== this.instanceId) { + return { ok: false, status: "stale", message: "Approval request is stale." }; + } + const route = this.routes.get(request.sessionKey); + if (!route?.actions.resolveApprovalDecision) { + return { ok: false, status: "stale", message: "Approval target is offline or stale." }; + } + const binding = await this.store.getActiveChannelBindingForSession(DISCORD_CHANNEL, request.sessionKey, { + instanceId: this.instanceId, + conversationId: message.conversation.id, + userId: message.sender.userId, + includePaused: true, + }); + if (!binding) { + return { ok: false, status: "unauthorized", message: "This Discord action is not authorized." }; + } + return route.actions.resolveApprovalDecision({ + approvalId, + decision, channel: DISCORD_CHANNEL, instanceId: this.instanceId, - conversationId: action.conversation.id, - userId: action.sender.userId, + conversationId: message.conversation.id, + userId: message.sender.userId, }); - await this.adapter?.answerAction(action.actionId, { text: result.message, alert: !result.ok }); + } + + private async handleApprovalCommand(message: ChannelInboundMessage, command: DiscordCommand): Promise { + const approvalCommand = parseApprovalTextCommand(command.name, command.args); + if (!approvalCommand) return false; + if (!this.config.discord || !isDiscordIdentityAllowed(message.sender, this.config.discord)) { + await this.sendText(message, "This Discord identity is not authorized to control this PiRelay machine bot."); + return true; + } + const result = await this.resolveApprovalDecisionFromDiscord(message, approvalCommand.approvalId, approvalCommand.decision); + await this.sendText(message, result.message); return true; } @@ -716,16 +752,7 @@ export class DiscordRuntime { } private async handleCommand(message: ChannelInboundMessage, binding: ChannelPersistedBindingRecord, route: SessionRoute, command: DiscordCommand): Promise { - const approvalCommand = parseApprovalTextCommand(command.name, command.args); - if (approvalCommand) { - if (!route.actions.resolveApprovalDecision) { - await this.sendText(message, "Approval request is stale."); - return; - } - const result = await route.actions.resolveApprovalDecision({ approvalId: approvalCommand.approvalId, decision: approvalCommand.decision, channel: DISCORD_CHANNEL, instanceId: this.instanceId, conversationId: message.conversation.id, userId: message.sender.userId }); - await this.sendText(message, result.message); - return; - } + if (await this.handleApprovalCommand(message, command)) return; switch (command.name) { case "help": await this.sendText(message, DISCORD_HELP_TEXT); diff --git a/extensions/relay/adapters/slack/runtime.ts b/extensions/relay/adapters/slack/runtime.ts index 3786fe1..3fd2e4c 100644 --- a/extensions/relay/adapters/slack/runtime.ts +++ b/extensions/relay/adapters/slack/runtime.ts @@ -19,7 +19,7 @@ import { SlackChannelAdapter, isSlackIdentityAllowed, slackEnvelopeToChannelEven import { createSlackLiveOperations, type SlackMessageEventFromHistory } from "./live-client.js"; import { delegationCommandFromAction, delegationIngressEventKey, delegationRoomFromMessage, evaluateDelegationIngress, isPeerBotIdentity } from "../../core/agent-delegation-runtime.js"; import { transitionDelegationTask, type DelegationTaskRecord } from "../../core/agent-delegation.js"; -import { parseApprovalActionData, parseApprovalTextCommand } from "../../core/approval-gates.js"; +import { parseApprovalActionData, parseApprovalTextCommand, type ApprovalDecisionKind, type ApprovalDecisionResult } from "../../core/approval-gates.js"; const SLACK_CHANNEL = "slack" as const; const SLACK_HELP_TEXT = buildHelpText({ @@ -372,22 +372,59 @@ export class SlackRuntime { private async handleApprovalAction(action: ChannelInboundAction): Promise { const parsed = parseApprovalActionData(action.actionData); if (!parsed) return false; - const binding = await this.findSlackBinding(action); - const route = binding ? this.routes.get(binding.sessionKey) : undefined; - if (!binding || !route?.actions.resolveApprovalDecision || !this.adapter) { - await this.adapter?.answerAction(action.actionId, { text: "Approval request is stale." }); + const slackConfig = this.configForInstance(); + if (!this.adapter || !slackConfig || !isSlackIdentityAllowed(action.sender, slackConfig)) { + await this.adapter?.answerAction(action.actionId, { text: "This Slack identity is not authorized to control PiRelay." }); return true; } - const result = await route.actions.resolveApprovalDecision({ - approvalId: parsed.approvalId, - decision: parsed.decision, + const result = await this.resolveApprovalDecisionFromSlack(action, parsed.approvalId, parsed.decision); + await this.adapter.answerAction(action.actionId, { text: result.message }); + return true; + } + + private async resolveApprovalDecisionFromSlack( + message: Pick, + approvalId: string, + decision: ApprovalDecisionKind, + ): Promise { + const request = await this.store.getApprovalRequest(approvalId); + if (!request || request.requester.channel !== SLACK_CHANNEL || (request.requester.instanceId ?? "default") !== this.instanceId) { + return { ok: false, status: "stale", message: "Approval request is stale." }; + } + const route = this.routes.get(request.sessionKey); + if (!route?.actions.resolveApprovalDecision) { + return { ok: false, status: "stale", message: "Approval target is offline or stale." }; + } + const binding = await this.store.getActiveChannelBindingForSession(SLACK_CHANNEL, request.sessionKey, { + instanceId: this.instanceId, + conversationId: message.conversation.id, + userId: message.sender.userId, + includePaused: true, + }); + if (!binding) { + return { ok: false, status: "unauthorized", message: "This Slack identity is not authorized to control PiRelay." }; + } + return route.actions.resolveApprovalDecision({ + approvalId, + decision, channel: SLACK_CHANNEL, instanceId: this.instanceId, - conversationId: action.conversation.id, - userId: action.sender.userId, - threadId: typeof action.metadata?.threadTs === "string" ? action.metadata.threadTs : undefined, + conversationId: message.conversation.id, + userId: message.sender.userId, + threadId: typeof message.metadata?.threadTs === "string" ? message.metadata.threadTs : undefined, }); - await this.adapter.answerAction(action.actionId, { text: result.message }); + } + + private async handleApprovalCommand(message: ChannelInboundMessage, command: SlackCommand): Promise { + const approvalCommand = parseApprovalTextCommand(command.name, command.args); + if (!approvalCommand) return false; + const slackConfig = this.configForInstance(); + if (!slackConfig || !isSlackIdentityAllowed(message.sender, slackConfig)) { + await this.sendText(message, "This Slack identity is not authorized to control PiRelay."); + return true; + } + const result = await this.resolveApprovalDecisionFromSlack(message, approvalCommand.approvalId, approvalCommand.decision); + await this.sendText(message, result.message); return true; } @@ -472,6 +509,8 @@ export class SlackRuntime { } if (isPeerBotIdentity(message.sender)) return; if (!slackConfig || !isSlackIdentityAllowed(message.sender, slackConfig)) return; + const approvalCommand = parseSlackCommand(routedMessage.text); + if (approvalCommand && await this.handleApprovalCommand(routedMessage, approvalCommand)) return; const binding = await this.findSlackBinding(routedMessage) ?? await this.livePreseededBinding(routedMessage); if (!binding) { await this.sendText(message, message.conversation.kind === "private" @@ -745,16 +784,7 @@ export class SlackRuntime { } private async handleSlackCommand(message: ChannelInboundMessage, binding: ChannelPersistedBindingRecord, route: SessionRoute, command: SlackCommand): Promise { - const approvalCommand = parseApprovalTextCommand(command.name, command.args); - if (approvalCommand) { - if (!route.actions.resolveApprovalDecision) { - await this.sendText(message, "Approval request is stale."); - return; - } - const result = await route.actions.resolveApprovalDecision({ approvalId: approvalCommand.approvalId, decision: approvalCommand.decision, channel: SLACK_CHANNEL, instanceId: this.instanceId, conversationId: message.conversation.id, userId: message.sender.userId, threadId: typeof message.metadata?.threadTs === "string" ? message.metadata.threadTs : undefined }); - await this.sendText(message, result.message); - return; - } + if (await this.handleApprovalCommand(message, command)) return; if (binding.paused && !commandAllowsWhilePaused(command.name)) { await this.sendText(message, "Remote delivery is paused for this Slack binding. Use `relay resume` first."); return; diff --git a/extensions/relay/adapters/telegram/runtime.ts b/extensions/relay/adapters/telegram/runtime.ts index 1059b92..361979b 100644 --- a/extensions/relay/adapters/telegram/runtime.ts +++ b/extensions/relay/adapters/telegram/runtime.ts @@ -65,7 +65,7 @@ import { formatFullOutput, formatRelayStatusForRoute, formatSessionSelectorError import { commandIntentFromPipeline, runTelegramIngressPipeline, telegramActionFromPipelineResult } from "./middleware.js"; import { delegationIngressEventKey, delegationRoomFromMessage, evaluateDelegationIngress } from "../../core/agent-delegation-runtime.js"; import { transitionDelegationTask, type DelegationTaskRecord } from "../../core/agent-delegation.js"; -import { parseApprovalActionData, parseApprovalTextCommand } from "../../core/approval-gates.js"; +import { parseApprovalActionData, parseApprovalTextCommand, type ApprovalDecisionKind, type ApprovalDecisionResult } from "../../core/approval-gates.js"; import { appendRecentActivity, displayProgressMode, @@ -722,6 +722,12 @@ export class InProcessTunnelRuntime implements TunnelRuntime { await this.handleAuthorizedCommand(undefined, message, command.command, command.args); return; } + const approvalCommand = command ? parseApprovalTextCommand(command.command, command.args) : undefined; + if (approvalCommand) { + const result = await this.resolveApprovalDecisionFromTelegram(message.chat.id, message.user, approvalCommand.approvalId, approvalCommand.decision); + await this.api.sendPlainText(message.chat.id, result.message); + return; + } const bindingSnapshot = await this.store.loadBindingAuthoritySnapshot(); if (bindingSnapshot.kind === "state-unavailable") { @@ -829,24 +835,7 @@ export class InProcessTunnelRuntime implements TunnelRuntime { private async processCallback(callback: TelegramInboundCallback): Promise { const approvalAction = parseApprovalActionData(callback.data); if (approvalAction) { - const binding = await this.activeBindingForMessage(callback.chat.id, callback.user.id); - const route = binding ? this.routes.get(binding.sessionKey) : undefined; - if (!route?.actions.resolveApprovalDecision) { - await this.api.answerCallbackQuery(callback.callbackQueryId, "Approval request is stale."); - return; - } - if (!route.binding || !(await this.isAuthorized(route, callback.user))) { - await this.api.answerCallbackQuery(callback.callbackQueryId, "Unauthorized."); - return; - } - const result = await route.actions.resolveApprovalDecision({ - approvalId: approvalAction.approvalId, - decision: approvalAction.decision, - channel: "telegram", - instanceId: "default", - conversationId: String(callback.chat.id), - userId: String(callback.user.id), - }); + const result = await this.resolveApprovalDecisionFromTelegram(callback.chat.id, callback.user, approvalAction.approvalId, approvalAction.decision); await this.api.answerCallbackQuery(callback.callbackQueryId, result.message); return; } @@ -1707,13 +1696,9 @@ export class InProcessTunnelRuntime implements TunnelRuntime { args: string, ): Promise { const approvalCommand = parseApprovalTextCommand(command, args); - if (approvalCommand && route?.actions.resolveApprovalDecision) { - const result = await route.actions.resolveApprovalDecision({ approvalId: approvalCommand.approvalId, decision: approvalCommand.decision, channel: "telegram", instanceId: "default", conversationId: String(message.chat.id), userId: String(message.user.id) }); - await this.api.sendPlainText(message.chat.id, result.message); - return; - } if (approvalCommand) { - await this.api.sendPlainText(message.chat.id, "Approval request is stale."); + const result = await this.resolveApprovalDecisionFromTelegram(message.chat.id, message.user, approvalCommand.approvalId, approvalCommand.decision); + await this.api.sendPlainText(message.chat.id, result.message); return; } if (command === "help") { @@ -1936,6 +1921,35 @@ export class InProcessTunnelRuntime implements TunnelRuntime { } } + private async resolveApprovalDecisionFromTelegram( + chatId: number, + user: TelegramUserSummary, + approvalId: string, + decision: ApprovalDecisionKind, + ): Promise { + const request = await this.store.getApprovalRequest(approvalId); + if (!request || request.requester.channel !== "telegram") { + return { ok: false, status: "stale", message: "Approval request is stale." }; + } + const route = this.routes.get(request.sessionKey); + if (!route?.actions.resolveApprovalDecision) { + return { ok: false, status: "stale", message: "Approval target is offline or stale." }; + } + const binding = await this.store.getActiveBindingForSession(request.sessionKey, { chatId, userId: user.id, includePaused: true }); + if (binding) route.binding = binding; + if (!route.binding || !(await this.isAuthorized(route, user))) { + return { ok: false, status: "unauthorized", message: "Unauthorized." }; + } + return route.actions.resolveApprovalDecision({ + approvalId, + decision, + channel: "telegram", + instanceId: "default", + conversationId: String(chatId), + userId: String(user.id), + }); + } + private async deliverPlainPrompt(route: SessionRoute, message: TelegramInboundMessage, text: string): Promise { this.clearAnswerStateForRoute(route); const delivery = this.promptDeliveryState(route); diff --git a/extensions/relay/broker/process.js b/extensions/relay/broker/process.js index 06ca062..5accae6 100644 --- a/extensions/relay/broker/process.js +++ b/extensions/relay/broker/process.js @@ -383,10 +383,16 @@ async function loadStateSnapshot() { activeChannelSelections: parsed.activeChannelSelections || {}, trustedRelayUsers: parsed.trustedRelayUsers || {}, lifecycleNotifications: parsed.lifecycleNotifications || {}, + delegationTasks: parsed.delegationTasks || {}, + delegationAudit: parsed.delegationAudit || [], + delegationHandledEvents: parsed.delegationHandledEvents || [], + approvalRequests: parsed.approvalRequests || {}, + approvalGrants: parsed.approvalGrants || {}, + approvalAudit: parsed.approvalAudit || [], }); } catch (error) { if (error && typeof error === 'object' && error.code === 'ENOENT') { - return bindingAuthorityStateFromData({ pendingPairings: {}, bindings: {}, channelBindings: {}, activeChannelSelections: {}, trustedRelayUsers: {}, lifecycleNotifications: {} }, { missing: true }); + return bindingAuthorityStateFromData({ pendingPairings: {}, bindings: {}, channelBindings: {}, activeChannelSelections: {}, trustedRelayUsers: {}, lifecycleNotifications: {}, delegationTasks: {}, delegationAudit: [], delegationHandledEvents: [], approvalRequests: {}, approvalGrants: {}, approvalAudit: [] }, { missing: true }); } return stateUnavailableBindingAuthority(error); } @@ -394,7 +400,19 @@ async function loadStateSnapshot() { async function loadState() { const snapshot = await loadStateSnapshot(); - return snapshot.kind === 'loaded' ? snapshot.data : { pendingPairings: {}, bindings: {}, channelBindings: {}, activeChannelSelections: {}, trustedRelayUsers: {}, lifecycleNotifications: {} }; + return snapshot.kind === 'loaded' ? snapshot.data : { pendingPairings: {}, bindings: {}, channelBindings: {}, activeChannelSelections: {}, trustedRelayUsers: {}, lifecycleNotifications: {}, delegationTasks: {}, delegationAudit: [], delegationHandledEvents: [], approvalRequests: {}, approvalGrants: {}, approvalAudit: [] }; +} + +async function loadApprovalRequest(approvalId) { + try { + await mkdir(config.stateDir, { recursive: true, mode: 0o700 }); + const raw = await readFile(statePath, 'utf8'); + const parsed = JSON.parse(raw); + return parsed.approvalRequests?.[approvalId]; + } catch (error) { + if (error && typeof error === 'object' && error.code === 'ENOENT') return undefined; + throw error; + } } async function saveState(state) { @@ -566,6 +584,19 @@ async function resolveRouteForChat(chatId, userId) { return { route: undefined, live, ambiguous: false }; } +async function resolveApprovalRouteForTelegram(approvalId, chatId, userId) { + const request = await loadApprovalRequest(approvalId); + if (!request || request.requester?.channel !== 'telegram') return undefined; + const route = routes.get(request.sessionKey); + if (!route) return undefined; + const state = await loadState(); + const persisted = state.bindings?.[request.sessionKey]; + if (persisted?.status !== 'revoked' && persisted.chatId === chatId && persisted.userId === userId) { + route.binding = persisted; + } + return route; +} + async function resolveRouteSelectorForChat(chatId, userId, selector) { const entries = await getSessionEntriesForChat(chatId, userId); const result = resolveSessionSelector(entries, selector); @@ -1510,12 +1541,13 @@ async function handleAuthorizedCommand(message, route, command, args) { const binding = route?.binding; const approvalCommand = parseApprovalTextCommand(command, args || ''); if (approvalCommand) { - if (!route || !binding) { - await sendPlainText(message.chat.id, 'Approval request is stale.'); + const approvalRoute = await resolveApprovalRouteForTelegram(approvalCommand.approvalId, message.chat.id, message.user.id); + if (!approvalRoute || !(await routeIsAuthorized(approvalRoute, message.user))) { + await sendPlainText(message.chat.id, approvalRoute ? 'Unauthorized.' : 'Approval request is stale.'); return; } try { - const result = await requestClient(route, 'resolveApprovalDecision', { + const result = await requestClient(approvalRoute, 'resolveApprovalDecision', { decision: { approvalId: approvalCommand.approvalId, decision: approvalCommand.decision, @@ -1816,7 +1848,8 @@ async function processInbound(message) { await sendPlainText(message.chat.id, 'Unauthorized Telegram identity for this Pi session.'); return; } - if (ambiguous && command?.command !== 'sessions' && command?.command !== 'use' && command?.command !== 'forget' && command?.command !== 'to') { + const approvalCommand = command ? parseApprovalTextCommand(command.command, command.args || '') : undefined; + if (ambiguous && !approvalCommand && command?.command !== 'sessions' && command?.command !== 'use' && command?.command !== 'forget' && command?.command !== 'to') { await sendPlainText(message.chat.id, 'Multiple Pi sessions are paired to this chat. Use /sessions then /use first.'); return; } @@ -1913,8 +1946,7 @@ async function handleDashboardAction(callback, route, action) { async function processCallback(callback) { const approvalAction = parseApprovalActionData(callback.data || ''); if (approvalAction) { - const live = await getActiveLiveRoutesForChat(callback.chat.id, callback.user.id); - const route = live.find((candidate) => candidate.sessionKey === activeSessionByChatId.get(String(callback.chat.id))) || live[0]; + const route = await resolveApprovalRouteForTelegram(approvalAction.approvalId, callback.chat.id, callback.user.id); if (!route || !(await routeIsAuthorized(route, callback.user))) { await answerCallbackQuery(callback.callbackQueryId, route ? 'Unauthorized.' : 'Approval request is stale.'); return; diff --git a/extensions/relay/core/approval-gates.ts b/extensions/relay/core/approval-gates.ts index 99e221e..a48b59e 100644 --- a/extensions/relay/core/approval-gates.ts +++ b/extensions/relay/core/approval-gates.ts @@ -126,6 +126,19 @@ export const MIN_APPROVAL_TIMEOUT_MS = 5_000; export const MAX_APPROVAL_TIMEOUT_MS = 30 * 60_000; export const MAX_APPROVAL_SUMMARY_CHARS = 700; const ACTION_PREFIX = "pirelay:approval"; +const COMPACT_ACTION_PREFIX = "pra"; +const APPROVAL_DECISION_CODES: Record = { + "approve-once": "ao", + "approve-session": "as", + "approve-persistent": "ap", + deny: "d", +}; +const APPROVAL_DECISION_BY_CODE: Record = { + ao: "approve-once", + as: "approve-session", + ap: "approve-persistent", + d: "deny", +}; export function resolveApprovalGateConfig(config: ApprovalGateConfig | undefined): ResolvedApprovalGateConfig { const enabled = config?.enabled === true; @@ -216,7 +229,7 @@ export function approvalMatcherFingerprint(input: { toolName: string; category: } export function approvalActionData(decision: ApprovalDecisionKind, approvalId: string): string { - return `${ACTION_PREFIX}:${decision}:${approvalId}`; + return `${COMPACT_ACTION_PREFIX}:${APPROVAL_DECISION_CODES[decision]}:${approvalId}`; } export function parseApprovalTextCommand(command: string, args: string): { decision: ApprovalDecisionKind; approvalId: string } | undefined { @@ -235,6 +248,11 @@ export function parseApprovalTextCommand(command: string, args: string): { decis export function parseApprovalActionData(value: string): { decision: ApprovalDecisionKind; approvalId: string } | undefined { const parts = value.split(":"); + if (parts.length === 3 && parts[0] === COMPACT_ACTION_PREFIX) { + const decision = APPROVAL_DECISION_BY_CODE[parts[1] ?? ""]; + const approvalId = parts[2]?.trim(); + return decision && approvalId ? { decision, approvalId } : undefined; + } if (parts.length !== 4 || parts[0] !== "pirelay" || parts[1] !== "approval") return undefined; const decision = parts[2]; if (decision !== "approve-once" && decision !== "approve-session" && decision !== "approve-persistent" && decision !== "deny") return undefined; @@ -341,12 +359,18 @@ function ruleMatches(rule: ApprovalGateRule, input: { toolName: string; category const tools = [...(rule.tools ?? []), ...(rule.toolNames ?? [])].map((tool) => tool.toLowerCase()); if (tools.length > 0 && !tools.includes(input.toolName.toLowerCase())) return false; if (rule.categories && rule.categories.length > 0 && !rule.categories.includes(input.category)) return false; - if (patternsMatch(rule.commandPatterns, input.commandText)) return true; - if (patternsMatch(rule.pathPatterns, input.pathText)) return true; - if (patternsMatch(rule.textPatterns, input.searchable)) return true; + const hasPatternConstraints = hasConfiguredPatterns(rule.commandPatterns) || hasConfiguredPatterns(rule.pathPatterns) || hasConfiguredPatterns(rule.textPatterns); + const patternMatched = patternsMatch(rule.commandPatterns, input.commandText) + || patternsMatch(rule.pathPatterns, input.pathText) + || patternsMatch(rule.textPatterns, input.searchable); + if (hasPatternConstraints) return patternMatched; return tools.length > 0 || Boolean(rule.categories && rule.categories.length > 0); } +function hasConfiguredPatterns(patterns: string[] | undefined): boolean { + return Boolean(patterns?.some((pattern) => pattern.trim().length > 0)); +} + function patternsMatch(patterns: string[] | undefined, text: string): boolean { if (!patterns || patterns.length === 0) return false; return patterns.some((pattern) => { diff --git a/extensions/relay/runtime/extension-runtime.ts b/extensions/relay/runtime/extension-runtime.ts index ee4a006..6d2dbed 100644 --- a/extensions/relay/runtime/extension-runtime.ts +++ b/extensions/relay/runtime/extension-runtime.ts @@ -754,8 +754,7 @@ export default function telegramTunnelExtension(pi: ExtensionAPI): void { return timeout; } - async function resolveApprovalDecision(decision: ApprovalDecisionRequest): Promise { - const route = currentRoute; + async function resolveApprovalDecision(route: SessionRoute, decision: ApprovalDecisionRequest): Promise { const config = configCache; const store = approvalStore(config); if (!route || !config || !store) return { ok: false, status: "stale", message: "Approval target is offline or stale." }; @@ -934,7 +933,7 @@ export default function telegramTunnelExtension(pi: ExtensionAPI): void { throw error; } }, - resolveApprovalDecision, + resolveApprovalDecision: (decision) => resolveApprovalDecision(route, decision), compact: () => new Promise((resolve, reject) => { const live = liveContextForRoute(route); diff --git a/tests/discord-runtime.test.ts b/tests/discord-runtime.test.ts index 3a14d35..f2d72f4 100644 --- a/tests/discord-runtime.test.ts +++ b/tests/discord-runtime.test.ts @@ -1432,7 +1432,34 @@ describe("DiscordRuntime", () => { await ops.handler?.({ type: "interaction", payload: { id: "i1", token: "t1", channel_id: "dm1", user: { id: "u1" }, data: { custom_id: "x" } } }); const resolveApprovalDecision = vi.fn(async () => ({ ok: true, status: "approved" as const, message: "Approved once." })); session.actions.resolveApprovalDecision = resolveApprovalDecision; - await ops.handler?.({ type: "interaction", payload: { id: "i-approval", token: "t-approval", channel_id: "dm1", user: { id: "u1" }, data: { custom_id: "pirelay:approval:approve-once:approval-1" } } }); + await store.upsertChannelBinding({ + channel: "discord", + conversationId: "dm1", + userId: "u1", + sessionKey: "other-session", + sessionId: "other-session", + sessionLabel: "Other", + boundAt: new Date().toISOString(), + lastSeenAt: new Date().toISOString(), + }); + await store.setActiveChannelSelection("discord", "dm1", "u1", "other-session"); + await store.upsertApprovalRequest({ + approvalId: "approval-1", + sessionKey: session.sessionKey, + sessionLabel: session.sessionLabel, + operationId: "tool-1", + toolName: "bash", + category: "shell", + safeSummary: "Run shell command", + matcherFingerprint: "shell:bash:test", + requester: { channel: "discord", instanceId: "default", conversationId: "dm1", userId: "u1", sessionKey: session.sessionKey, safeLabel: "Discord u1", createdAt: Date.now() }, + createdAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 60_000).toISOString(), + status: "pending", + }); + await ops.handler?.({ type: "interaction", payload: { id: "i-approval", token: "t-approval", channel_id: "dm1", user: { id: "u1" }, data: { custom_id: "pra:ao:approval-1" } } }); + await store.revokeChannelBinding("discord", "other-session"); + await store.setActiveChannelSelection("discord", "dm1", "u1", session.sessionKey); await ops.handler?.({ type: "interaction", payload: { id: "i2", token: "t2", channel_id: "dm1", user: { id: "u2" }, data: { custom_id: "x" } } }); await store.upsertChannelBinding({ channel: "discord", diff --git a/tests/relay/approval-gates.test.ts b/tests/relay/approval-gates.test.ts index f4a70f4..bc9b436 100644 --- a/tests/relay/approval-gates.test.ts +++ b/tests/relay/approval-gates.test.ts @@ -49,10 +49,12 @@ describe("approval gates", () => { expect(operation?.matcherFingerprint).toContain("git-remote:bash:git"); }); - it("matches write/edit path patterns and custom text patterns", () => { - const config = resolveApprovalGateConfig({ enabled: true, rules: [{ id: "protected", pathPatterns: ["package.json"] }, { id: "custom", tools: ["deploy"], textPatterns: ["prod"] }] }); + it("matches write/edit path patterns and custom text patterns only when constraints match", () => { + const config = resolveApprovalGateConfig({ enabled: true, rules: [{ id: "protected", tools: ["write", "edit"], pathPatterns: ["package.json"] }, { id: "custom", tools: ["deploy"], textPatterns: ["prod"] }] }); expect(classifyApprovalOperation({ toolName: "write", input: { path: "package.json", content: "{}" } }, config)?.category).toBe("file-write"); + expect(classifyApprovalOperation({ toolName: "write", input: { path: "README.md", content: "{}" } }, config)).toBeUndefined(); expect(classifyApprovalOperation({ toolName: "deploy", input: { target: "prod" } }, config)?.category).toBe("custom"); + expect(classifyApprovalOperation({ toolName: "deploy", input: { target: "dev" } }, config)).toBeUndefined(); }); it("redacts and bounds approval summaries", () => { @@ -69,7 +71,10 @@ describe("approval gates", () => { expect(renderApprovalRequest(request, config)).toContain("Approval required"); expect(renderApprovalRequest(request, config)).not.toContain("npm-token"); expect(approvalButtons(request, config).flat().map((button) => button.label)).toEqual(["Approve once", "Approve for session", "Approve persistent", "Deny"]); - expect(parseApprovalActionData(approvalActionData("approve-once", request.approvalId))).toEqual({ decision: "approve-once", approvalId: request.approvalId }); + const actionData = approvalActionData("approve-persistent", request.approvalId); + expect(actionData.length).toBeLessThanOrEqual(64); + expect(parseApprovalActionData(actionData)).toEqual({ decision: "approve-persistent", approvalId: request.approvalId }); + expect(parseApprovalActionData(`pirelay:approval:approve-once:${request.approvalId}`)).toEqual({ decision: "approve-once", approvalId: request.approvalId }); }); it("matches session grants only for same requester, session, fingerprint, and expiry", () => { diff --git a/tests/runtime.test.ts b/tests/runtime.test.ts index 9442a79..5c59fb0 100644 --- a/tests/runtime.test.ts +++ b/tests/runtime.test.ts @@ -1235,7 +1235,28 @@ describe("InProcessTunnelRuntime", () => { const resolveApprovalDecision = vi.fn(async () => ({ ok: true, status: "approved" as const, message: "Approved once." })); route.actions.resolveApprovalDecision = resolveApprovalDecision; await store.upsertBinding(binding); + await store.upsertApprovalRequest({ + approvalId: "approval-1", + sessionKey: binding.sessionKey, + sessionLabel: binding.sessionLabel, + operationId: "tool-1", + toolName: "bash", + category: "shell", + safeSummary: "Run shell command", + matcherFingerprint: "shell:bash:test", + requester: { channel: "telegram", instanceId: "default", conversationId: String(binding.chatId), userId: String(binding.userId), sessionKey: binding.sessionKey, safeLabel: "owner", createdAt: Date.now() }, + createdAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 60_000).toISOString(), + status: "pending", + }); (runtime as any).routes.set(route.sessionKey, route); + const otherBinding: TelegramBindingMetadata = { ...binding, sessionKey: "other-approval-session:/tmp/other.jsonl", sessionId: "other-approval-session", sessionFile: "/tmp/other.jsonl", sessionLabel: "other.jsonl" }; + const { route: otherRoute } = createRoute(otherBinding, true); + const otherResolveApprovalDecision = vi.fn(async () => ({ ok: false, status: "stale" as const, message: "Wrong route." })); + otherRoute.actions.resolveApprovalDecision = otherResolveApprovalDecision; + await store.upsertBinding(otherBinding); + (runtime as any).routes.set(otherRoute.sessionKey, otherRoute); + (runtime as any).activeSessionByChatUser.set("10022:220", otherRoute.sessionKey); const callbacks: string[] = []; (runtime as any).api = { sendPlainText: async () => undefined, @@ -1253,6 +1274,7 @@ describe("InProcessTunnelRuntime", () => { }); expect(resolveApprovalDecision).toHaveBeenCalledWith(expect.objectContaining({ approvalId: "approval-1", decision: "approve-once", channel: "telegram", conversationId: "10022", userId: "220" })); + expect(otherResolveApprovalDecision).not.toHaveBeenCalled(); expect(callbacks).toEqual(["Approved once."]); }); diff --git a/tests/slack-runtime.test.ts b/tests/slack-runtime.test.ts index e551bf0..b37aa27 100644 --- a/tests/slack-runtime.test.ts +++ b/tests/slack-runtime.test.ts @@ -670,7 +670,43 @@ describe("SlackRuntime foundations", () => { const resolveApprovalDecision = vi.fn(async () => ({ ok: true, status: "approved" as const, message: "Approved once." })); testRoute.actions.resolveApprovalDecision = resolveApprovalDecision; - await operations.handler!({ type: "block_actions", channel: { id: "D1" }, user: { id: "U_DRIVER", team_id: "T1" }, actions: [{ value: "pirelay:approval:approve-once:approval-1" }], response_url: "https://hooks.slack.test/approval" }); + await store.upsertChannelBinding({ + channel: "slack", + instanceId: "default", + conversationId: "D1", + userId: "U_DRIVER", + sessionKey: "other-session", + sessionId: "other-session", + sessionLabel: "Other", + boundAt: new Date().toISOString(), + lastSeenAt: new Date().toISOString(), + }); + await store.setActiveChannelSelection("slack", "D1", "U_DRIVER", "other-session"); + await store.trustRelayUser({ + channel: "slack", + instanceId: "default", + userId: "U_BLOCKED", + trustedBySessionLabel: testRoute.sessionLabel, + }); + await store.upsertApprovalRequest({ + approvalId: "approval-1", + sessionKey: testRoute.sessionKey, + sessionLabel: testRoute.sessionLabel, + operationId: "tool-1", + toolName: "bash", + category: "shell", + safeSummary: "Run shell command", + matcherFingerprint: "shell:bash:test", + requester: { channel: "slack", instanceId: "default", conversationId: "D1", userId: "U_DRIVER", sessionKey: testRoute.sessionKey, safeLabel: "Slack U_DRIVER", createdAt: Date.now() }, + createdAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 60_000).toISOString(), + status: "pending", + }); + await operations.handler!({ type: "block_actions", trigger_id: "blocked-approval-trigger", channel: { id: "D1" }, user: { id: "U_BLOCKED", team_id: "T1" }, actions: [{ value: "pra:ao:approval-1" }], response_url: "https://hooks.slack.test/blocked-approval" }); + expect(resolveApprovalDecision).not.toHaveBeenCalled(); + expect(operations.responses.at(-1)?.text).toBe("This Slack identity is not authorized to control PiRelay."); + await operations.handler!({ type: "block_actions", trigger_id: "allowed-approval-trigger", channel: { id: "D1" }, user: { id: "U_DRIVER", team_id: "T1" }, actions: [{ value: "pra:ao:approval-1" }], response_url: "https://hooks.slack.test/approval" }); + await store.setActiveChannelSelection("slack", "D1", "U_DRIVER", testRoute.sessionKey); expect(resolveApprovalDecision).toHaveBeenCalledWith(expect.objectContaining({ approvalId: "approval-1", decision: "approve-once", channel: "slack", conversationId: "D1", userId: "U_DRIVER" })); expect(operations.responses.at(-1)?.text).toBe("Approved once."); await send("/disconnect", "54");