mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-07-09 18:57:08 +08:00
feat: common approval contract — eligibility, hold records, sender fold (guarded-actions phase 1)
One hold-record contract over the existing tables and ONE click-authorization rule for every approval stack (hub: engineering/discovery/guarded-actions-decisions + engineering/requirements/guarded-actions, phase 1). - ApproverEligibility (exclusive | admins-of-scope) + mayResolve() replace the three divergent click-auth copies; a2a named approvers stay exclusive, sender/channel keep named-or-admin — the hold row encodes which. - pending_approvals gains eligibility / approver_scope / dedup_key (migration 019, with backfills); requestApproval stamps agent_group_id, supports sessionless holds, per-card options, dedup keys, and blast-radius scope. - Sender admission folds onto the primitive (action 'sender_admit'): addMember + routeInbound replay on approve; pending_sender_approvals and its card/click code are deleted. - Channel registration + OneCLI keep their flows, adopt eligibility/mayResolve; their terminal resolutions (click / expiry / boot sweep) announce through notifyApprovalResolved (outcome: approve|reject|expire|sweep, session nullable). - Absorbed defect fixes (intentional decision-outcome changes): D1 — global-blast holds (roles grant …) require an owner/global-admin click; D4 (approver half) — channel-registration approvers come from the global chain, not getAllAgentGroups()[0]. Everything else is behavior-preserving: card text/buttons, approver DM walk, reject-with-reason, OneCLI two-button card, oa- short ids, expiry timer and startup sweep are untouched; durable a2a holds stay durable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ All notable changes to NanoClaw will be documented in this file.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
- **One approval contract across every hold (guarded-actions phase 1).** All approval holds now share one hold-record shape on `pending_approvals` (eligibility rule, approver blast-radius scope, dedup key) and ONE click-authorization rule (`mayResolve` in `src/modules/approvals/eligibility.ts`), replacing three divergent click-auth copies. Unknown-sender admission folds onto the approvals primitive (sessionless hold, action `sender_admit`) — the `pending_sender_approvals` table is dropped by migration; an in-flight sender card does not survive the upgrade (a new message from the same sender re-triggers one). Channel registration and OneCLI keep their flows and adopt the shared eligibility; their resolutions (click / expiry / boot sweep) now announce through the approval-resolved observer. Two absorbed security fixes intentionally change decision outcomes: **(D1)** a hold with global blast radius (e.g. `roles grant`) can only be approved by an owner or global admin — a scoped admin's click is now rejected; **(D4, approver half)** channel-registration approvers are owners/global admins, no longer "admin of whichever agent group sorts first".
|
||||
- **Optional per-container resource caps.** `CONTAINER_CPU_LIMIT` and `CONTAINER_MEMORY_LIMIT` pass through to `docker run` as `--cpus` / `--memory` (`container-runner.ts`). Both empty by default — no flag added, spawn args byte-identical to today — so existing installs are unaffected. Set them to cap an agent container's CPU/memory so one agent can't monopolize the host (e.g. `CONTAINER_CPU_LIMIT=2`, `CONTAINER_MEMORY_LIMIT=8g`). Swap is intentionally not managed here: `--memory` is a hard cap on a swapless host.
|
||||
- [BREAKING] **Chat SDK pinned to `4.29.0` (was `4.26.0` via `^4.24.0`).** `chat` and the `@chat-adapter/*` channel adapters are version-locked — the adapter's `ChatInstance` must match the bridge's, so a mismatched pair fails to typecheck at `createChatSdkBridge(...)`. `chat` is therefore pinned exactly, and the channel-adapter install pins move with it — the `/add-<channel>` SKILL.md steps and `setup/*.sh` scripts on `main`, plus the adapter code on the `channels` branch. Core installs with no channel (only `cli`) are unaffected. **Migration:** if any channel is installed (Slack, Discord, Telegram, Teams, …), re-run its `/add-<channel>` skill to pull the matching `4.29.0` adapter.
|
||||
- **Budget/billing-exhausted LLM turns now reach the user instead of being silently dropped.** When a turn ends in a non-retryable provider error (e.g. an Anthropic `403 billing_error`) with no `<message>` wrapping, the agent-runner delivers the provider's notice to the originating channel and stops re-nudging the failing gateway. `providers/claude.ts` now surfaces the SDK's `is_error` flag (and the error subtype's `errors[]` text); `poll-loop.ts` delivers that text and skips the re-wrap retry. Fixes the case where a spend-limit notice produced silence plus a turn-after-turn retry loop.
|
||||
|
||||
@@ -136,6 +136,33 @@ register({
|
||||
},
|
||||
});
|
||||
|
||||
register({
|
||||
name: 'roles-grant',
|
||||
description: 'approval command on a non-scoped resource (global blast radius)',
|
||||
resource: 'roles',
|
||||
access: 'approval',
|
||||
parseArgs: (raw) => raw,
|
||||
handler: async () => ({}),
|
||||
});
|
||||
|
||||
register({
|
||||
name: 'members-add-gated',
|
||||
description: 'approval command on a scoped resource',
|
||||
resource: 'members',
|
||||
access: 'approval',
|
||||
parseArgs: (raw) => raw,
|
||||
handler: async () => ({}),
|
||||
});
|
||||
|
||||
register({
|
||||
name: 'groups-update',
|
||||
description: 'approval command on the groups resource (id = agent group)',
|
||||
resource: 'groups',
|
||||
access: 'approval',
|
||||
parseArgs: (raw) => raw,
|
||||
handler: async () => ({}),
|
||||
});
|
||||
|
||||
// Commands that return data shaped like real resources (for post-handler filtering tests)
|
||||
register({
|
||||
name: 'groups-list-data',
|
||||
@@ -473,6 +500,47 @@ describe('CLI scope enforcement', () => {
|
||||
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// --- Approver blast radius (D1) ---
|
||||
|
||||
it('holds on non-scoped resources carry approverScope global (roles grant)', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' });
|
||||
mockGetSession.mockReturnValue({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' });
|
||||
mockGetAgentGroup.mockReturnValue({ id: 'g1', name: 'Group One' });
|
||||
|
||||
const resp = await dispatch(
|
||||
{ id: '1', command: 'roles-grant', args: { user: 'telegram:mallory', role: 'owner' } },
|
||||
agentCtx(),
|
||||
);
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1);
|
||||
expect(approvalState.requestApproval.mock.calls[0][0]).toMatchObject({ approverScope: 'global' });
|
||||
});
|
||||
|
||||
it('holds on scoped resources pinned to the caller stay approverScope group', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
mockGetSession.mockReturnValue({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' });
|
||||
mockGetAgentGroup.mockReturnValue({ id: 'g1', name: 'Group One' });
|
||||
|
||||
const resp = await dispatch({ id: '1', command: 'members-add-gated', args: { user: 'telegram:new' } }, agentCtx());
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1);
|
||||
expect(approvalState.requestApproval.mock.calls[0][0]).toMatchObject({ approverScope: 'group' });
|
||||
});
|
||||
|
||||
it('holds on scoped resources targeting another group escalate to approverScope global', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' });
|
||||
mockGetSession.mockReturnValue({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' });
|
||||
mockGetAgentGroup.mockReturnValue({ id: 'g1', name: 'Group One' });
|
||||
|
||||
const resp = await dispatch({ id: '1', command: 'groups-update', args: { id: 'g2' } }, agentCtx());
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1);
|
||||
expect(approvalState.requestApproval.mock.calls[0][0]).toMatchObject({ approverScope: 'global' });
|
||||
});
|
||||
|
||||
// --- Post-handler filtering ---
|
||||
|
||||
it('group: groups list filters out other groups', async () => {
|
||||
|
||||
+27
-3
@@ -12,13 +12,37 @@ import { getSession } from '../db/sessions.js';
|
||||
import { registerApprovalHandler, requestApproval } from '../modules/approvals/index.js';
|
||||
import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js';
|
||||
import { getResource } from './crud.js';
|
||||
import { lookup } from './registry.js';
|
||||
import { lookup, type CommandDef } from './registry.js';
|
||||
|
||||
type DispatchOptions = {
|
||||
/** True when a command is being replayed after approval. */
|
||||
approved?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
* Resources reachable under `cli_scope: 'group'` — and, because their rows
|
||||
* anchor to one agent group, the resources whose held mutations have
|
||||
* group-local blast radius.
|
||||
*/
|
||||
const GROUP_SCOPED_RESOURCES = new Set(['groups', 'sessions', 'destinations', 'members']);
|
||||
|
||||
/**
|
||||
* Blast radius of a held command, for approver eligibility (D1): a mutation
|
||||
* of a non-group-scoped resource (roles, users, wirings, messaging-groups,
|
||||
* policies) — or one explicitly targeting another agent group — needs an
|
||||
* owner or global admin to approve; a scoped admin's click is rejected.
|
||||
*/
|
||||
function approverScopeFor(
|
||||
cmd: CommandDef,
|
||||
args: Record<string, unknown>,
|
||||
callerAgentGroupId: string,
|
||||
): 'group' | 'global' {
|
||||
if (!cmd.resource || !GROUP_SCOPED_RESOURCES.has(cmd.resource)) return 'global';
|
||||
const groupRefs = [args.agent_group_id, args.group];
|
||||
if (cmd.resource === 'groups' || cmd.resource === 'destinations') groupRefs.push(args.id);
|
||||
return groupRefs.some((v) => v !== undefined && v !== callerAgentGroupId) ? 'global' : 'group';
|
||||
}
|
||||
|
||||
export async function dispatch(
|
||||
req: RequestFrame,
|
||||
ctx: CallerContext,
|
||||
@@ -62,9 +86,8 @@ export async function dispatch(
|
||||
}
|
||||
|
||||
if (cliScope === 'group') {
|
||||
const allowed = new Set(['groups', 'sessions', 'destinations', 'members']);
|
||||
// Only allow whitelisted resources and general commands (no resource, like help)
|
||||
if (cmd.resource && !allowed.has(cmd.resource)) {
|
||||
if (cmd.resource && !GROUP_SCOPED_RESOURCES.has(cmd.resource)) {
|
||||
return err(req.id, 'forbidden', `CLI access is scoped to this agent group. Cannot access "${cmd.resource}".`);
|
||||
}
|
||||
|
||||
@@ -134,6 +157,7 @@ export async function dispatch(
|
||||
payload: { frame: { id: req.id, command: req.command, args: req.args }, callerContext: ctx },
|
||||
title: `CLI: ${req.command}`,
|
||||
question: `Agent "${agentName}" wants to run:\n\`ncl ${req.command}${argSummary ? ' ' + argSummary : ''}\``,
|
||||
approverScope: approverScopeFor(cmd, req.args, ctx.agentGroupId),
|
||||
});
|
||||
|
||||
return err(req.id, 'approval-pending', 'Approval request sent to admin. You will be notified of the result.');
|
||||
|
||||
@@ -48,6 +48,24 @@ registerResource({
|
||||
},
|
||||
{ name: 'title', type: 'string', description: 'Card title shown to the admin.' },
|
||||
{ name: 'options_json', type: 'json', description: 'Card button options as JSON array.' },
|
||||
{
|
||||
name: 'approver_user_id',
|
||||
type: 'string',
|
||||
description: 'Named approver (exclusive) or the admin the card was delivered to (admins-of-scope).',
|
||||
},
|
||||
{
|
||||
name: 'eligibility',
|
||||
type: 'string',
|
||||
description: 'Who may resolve: only the named approver, or the admin chain of the anchoring group.',
|
||||
enum: ['exclusive', 'admins-of-scope'],
|
||||
},
|
||||
{
|
||||
name: 'approver_scope',
|
||||
type: 'string',
|
||||
description: "Blast radius: 'global' holds require an owner or global admin to resolve.",
|
||||
enum: ['group', 'global'],
|
||||
},
|
||||
{ name: 'dedup_key', type: 'string', description: 'In-flight dedup key (e.g. sender admission per chat+sender).' },
|
||||
],
|
||||
operations: { list: 'open', get: 'open' },
|
||||
});
|
||||
|
||||
@@ -109,10 +109,13 @@ describe('groups CLI delete cascades dependent rows (#2525)', () => {
|
||||
VALUES (?, ?, 'req-1', 'cli_command', '{}', ?, ?, 'pending', '', '[]')`,
|
||||
).run('pa-1', SID, now(), GID);
|
||||
|
||||
// Sessionless sender-admission hold anchored to the group (the folded
|
||||
// pending_sender_approvals shape) — covered by the agent_group_id leg of
|
||||
// the pending_approvals cascade.
|
||||
db.prepare(
|
||||
`INSERT INTO pending_sender_approvals (id, messaging_group_id, agent_group_id, sender_identity, sender_name, original_message, approver_user_id, created_at)
|
||||
VALUES ('psa-1', ?, ?, 'tg:99', 'them', '{}', ?, ?)`,
|
||||
).run(MGID, GID, UID, now());
|
||||
`INSERT INTO pending_approvals (approval_id, session_id, request_id, action, payload, created_at, agent_group_id, status, title, options_json, dedup_key)
|
||||
VALUES (?, NULL, 'req-2', 'sender_admit', '{}', ?, ?, 'pending', '', '[]', 'sender_admit:mg:tg:99')`,
|
||||
).run('pa-2', now(), GID);
|
||||
|
||||
db.prepare(
|
||||
`INSERT INTO pending_channel_approvals (messaging_group_id, agent_group_id, original_message, approver_user_id, created_at)
|
||||
@@ -148,10 +151,9 @@ describe('groups CLI delete cascades dependent rows (#2525)', () => {
|
||||
expect(data.removed).toMatchObject({
|
||||
sessions: 1,
|
||||
pending_questions: 1,
|
||||
pending_approvals: 1,
|
||||
pending_approvals: 2,
|
||||
agent_destinations_owned: 1,
|
||||
agent_destinations_pointing: 0,
|
||||
pending_sender_approvals: 1,
|
||||
pending_channel_approvals: 1,
|
||||
messaging_group_agents: 1,
|
||||
agent_group_members: 1,
|
||||
@@ -167,7 +169,6 @@ describe('groups CLI delete cascades dependent rows (#2525)', () => {
|
||||
count('SELECT COUNT(*) AS c FROM pending_approvals WHERE agent_group_id = ? OR session_id = ?', GID, SID),
|
||||
).toBe(0);
|
||||
expect(count('SELECT COUNT(*) AS c FROM agent_destinations WHERE agent_group_id = ?', GID)).toBe(0);
|
||||
expect(count('SELECT COUNT(*) AS c FROM pending_sender_approvals WHERE agent_group_id = ?', GID)).toBe(0);
|
||||
expect(count('SELECT COUNT(*) AS c FROM pending_channel_approvals WHERE agent_group_id = ?', GID)).toBe(0);
|
||||
expect(count('SELECT COUNT(*) AS c FROM messaging_group_agents WHERE agent_group_id = ?', GID)).toBe(0);
|
||||
expect(count('SELECT COUNT(*) AS c FROM agent_group_members WHERE agent_group_id = ?', GID)).toBe(0);
|
||||
|
||||
@@ -124,7 +124,6 @@ registerResource({
|
||||
pending_approvals: 0,
|
||||
agent_destinations_owned: 0,
|
||||
agent_destinations_pointing: 0,
|
||||
pending_sender_approvals: 0,
|
||||
pending_channel_approvals: 0,
|
||||
messaging_group_agents: 0,
|
||||
agent_group_members: 0,
|
||||
@@ -153,9 +152,6 @@ registerResource({
|
||||
.run(groupId, groupId).changes;
|
||||
}
|
||||
counts.sessions = db.prepare('DELETE FROM sessions WHERE agent_group_id = ?').run(groupId).changes;
|
||||
counts.pending_sender_approvals = db
|
||||
.prepare('DELETE FROM pending_sender_approvals WHERE agent_group_id = ?')
|
||||
.run(groupId).changes;
|
||||
counts.pending_channel_approvals = db
|
||||
.prepare('DELETE FROM pending_channel_approvals WHERE agent_group_id = ?')
|
||||
.run(groupId).changes;
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
/**
|
||||
* Upgrade-path test for migration 019 (holds-eligibility): in-flight
|
||||
* pending_approvals rows created by the pre-contract code must come out with
|
||||
* the eligibility the old click-auth gave them, and the sender table must be
|
||||
* gone.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { closeDb, initTestDb, runMigrations } from './index.js';
|
||||
import { migrations } from './migrations/index.js';
|
||||
import type Database from 'better-sqlite3';
|
||||
|
||||
let db: Database.Database;
|
||||
|
||||
beforeEach(() => {
|
||||
db = initTestDb();
|
||||
// Everything up to — but not including — the holds-eligibility migration.
|
||||
runMigrations(
|
||||
db,
|
||||
migrations.filter((m) => m.name !== 'holds-eligibility'),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
closeDb();
|
||||
});
|
||||
|
||||
function hasTable(name: string): boolean {
|
||||
return db.prepare("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?").get(name) !== undefined;
|
||||
}
|
||||
|
||||
describe('migration 019 — holds-eligibility', () => {
|
||||
it('backfills eligibility and agent_group_id on in-flight rows and drops the sender table', () => {
|
||||
const now = new Date().toISOString();
|
||||
db.prepare("INSERT INTO agent_groups (id, name, folder, created_at) VALUES ('ag-1', 'One', 'one', ?)").run(now);
|
||||
db.prepare(
|
||||
"INSERT INTO sessions (id, agent_group_id, messaging_group_id, thread_id, created_at) VALUES ('sess-1', 'ag-1', NULL, NULL, ?)",
|
||||
).run(now);
|
||||
|
||||
// Legacy a2a hold: named approver ⇒ was exclusive.
|
||||
db.prepare(
|
||||
`INSERT INTO pending_approvals (approval_id, session_id, request_id, action, payload, created_at, approver_user_id, title, options_json)
|
||||
VALUES ('appr-a2a', 'sess-1', 'appr-a2a', 'a2a_message_gate', '{}', ?, 'tg:dana', '', '[]')`,
|
||||
).run(now);
|
||||
// Legacy cli hold: no approver, no agent_group_id — click-auth fell back
|
||||
// to the session's group; the backfill makes that anchoring explicit.
|
||||
db.prepare(
|
||||
`INSERT INTO pending_approvals (approval_id, session_id, request_id, action, payload, created_at, title, options_json)
|
||||
VALUES ('appr-cli', 'sess-1', 'appr-cli', 'cli_command', '{}', ?, '', '[]')`,
|
||||
).run(now);
|
||||
// Legacy OneCLI hold: sessionless, agent_group_id already stamped.
|
||||
db.prepare(
|
||||
`INSERT INTO pending_approvals (approval_id, session_id, request_id, action, payload, created_at, agent_group_id, title, options_json)
|
||||
VALUES ('oa-1', NULL, 'req-uuid', 'onecli_credential', '{}', ?, 'ag-1', '', '[]')`,
|
||||
).run(now);
|
||||
|
||||
expect(hasTable('pending_sender_approvals')).toBe(true);
|
||||
|
||||
runMigrations(db); // applies only holds-eligibility
|
||||
|
||||
const rows = db
|
||||
.prepare('SELECT approval_id, eligibility, approver_scope, agent_group_id FROM pending_approvals')
|
||||
.all() as Array<{ approval_id: string; eligibility: string; approver_scope: string; agent_group_id: string }>;
|
||||
const byId = Object.fromEntries(rows.map((r) => [r.approval_id, r]));
|
||||
|
||||
expect(byId['appr-a2a']).toMatchObject({
|
||||
eligibility: 'exclusive',
|
||||
approver_scope: 'group',
|
||||
agent_group_id: 'ag-1',
|
||||
});
|
||||
expect(byId['appr-cli']).toMatchObject({
|
||||
eligibility: 'admins-of-scope',
|
||||
approver_scope: 'group',
|
||||
agent_group_id: 'ag-1',
|
||||
});
|
||||
expect(byId['oa-1']).toMatchObject({ eligibility: 'admins-of-scope', agent_group_id: 'ag-1' });
|
||||
|
||||
expect(hasTable('pending_sender_approvals')).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,44 @@
|
||||
import type { Migration } from './index.js';
|
||||
|
||||
/**
|
||||
* The hold-record contract lands on `pending_approvals` (guarded-actions
|
||||
* phase 1 — see the guarded-actions decisions doc, decision 5):
|
||||
*
|
||||
* - `eligibility` — who may resolve the hold: 'exclusive' (only
|
||||
* `approver_user_id`, e.g. an a2a policy's named approver) or
|
||||
* 'admins-of-scope' (the admin chain of `agent_group_id`, plus the
|
||||
* specific user the card was delivered to when `approver_user_id` is
|
||||
* stamped — the sender/channel "named-or-admin" semantic).
|
||||
* - `approver_scope` — the action's blast radius: 'global' holds (e.g.
|
||||
* `roles grant`) can only be resolved by an owner or global admin; a
|
||||
* scoped admin's click is rejected.
|
||||
* - `dedup_key` — in-flight dedup: while a pending row carries a key, a
|
||||
* second request with the same key is dropped (replaces the sender
|
||||
* table's UNIQUE(messaging_group_id, sender_identity)).
|
||||
*
|
||||
* Backfills: rows with a named approver were exclusive before this column
|
||||
* existed; `agent_group_id` is stamped from the requesting session so
|
||||
* click-auth no longer needs the session fallback.
|
||||
*
|
||||
* `pending_sender_approvals` is dropped: sender admission now holds through
|
||||
* the approvals primitive (action 'sender_admit'). In-flight sender cards at
|
||||
* upgrade time die with the table — they are transient courtesy cards, and a
|
||||
* new message from the same sender re-triggers one.
|
||||
*/
|
||||
export const migration019: Migration = {
|
||||
version: 19,
|
||||
name: 'holds-eligibility',
|
||||
up(db) {
|
||||
db.exec(`ALTER TABLE pending_approvals ADD COLUMN eligibility TEXT NOT NULL DEFAULT 'admins-of-scope';`);
|
||||
db.exec(`ALTER TABLE pending_approvals ADD COLUMN approver_scope TEXT NOT NULL DEFAULT 'group';`);
|
||||
db.exec(`ALTER TABLE pending_approvals ADD COLUMN dedup_key TEXT;`);
|
||||
db.exec(`UPDATE pending_approvals SET eligibility = 'exclusive' WHERE approver_user_id IS NOT NULL;`);
|
||||
db.exec(
|
||||
`UPDATE pending_approvals
|
||||
SET agent_group_id = (SELECT s.agent_group_id FROM sessions s WHERE s.id = pending_approvals.session_id)
|
||||
WHERE agent_group_id IS NULL AND session_id IS NOT NULL;`,
|
||||
);
|
||||
db.exec(`DROP INDEX IF EXISTS idx_pending_sender_approvals_mg;`);
|
||||
db.exec(`DROP TABLE IF EXISTS pending_sender_approvals;`);
|
||||
},
|
||||
};
|
||||
@@ -17,6 +17,7 @@ import { migration016 } from './016-messaging-group-instance.js';
|
||||
import { moduleApprovalsPendingApprovals } from './module-approvals-pending-approvals.js';
|
||||
import { moduleApprovalsTitleOptions } from './module-approvals-title-options.js';
|
||||
import { migration018 } from './018-approvals-approver-user-id.js';
|
||||
import { migration019 } from './019-holds-eligibility.js';
|
||||
|
||||
export interface Migration {
|
||||
version: number;
|
||||
@@ -50,6 +51,7 @@ export const migrations: Migration[] = [
|
||||
migration014,
|
||||
migration015,
|
||||
migration016,
|
||||
migration019,
|
||||
];
|
||||
|
||||
/** Row shape of PRAGMA foreign_key_check. Child rowids are stable across a
|
||||
|
||||
@@ -133,21 +133,6 @@ CREATE TABLE pending_questions (
|
||||
created_at TEXT NOT NULL
|
||||
);
|
||||
|
||||
-- Pending approvals for unknown senders (unknown_sender_policy='request_approval').
|
||||
-- In-flight dedup via UNIQUE(messaging_group_id, sender_identity): a second
|
||||
-- message from the same unknown sender while a card is pending is silently
|
||||
-- dropped instead of spamming the admin.
|
||||
CREATE TABLE pending_sender_approvals (
|
||||
id TEXT PRIMARY KEY,
|
||||
messaging_group_id TEXT NOT NULL REFERENCES messaging_groups(id),
|
||||
agent_group_id TEXT NOT NULL REFERENCES agent_groups(id),
|
||||
sender_identity TEXT NOT NULL, -- namespaced user id (channel_type:handle)
|
||||
sender_name TEXT,
|
||||
original_message TEXT NOT NULL, -- JSON of the original InboundEvent
|
||||
approver_user_id TEXT NOT NULL,
|
||||
created_at TEXT NOT NULL,
|
||||
UNIQUE(messaging_group_id, sender_identity)
|
||||
);
|
||||
`;
|
||||
|
||||
/**
|
||||
|
||||
+15
-11
@@ -155,11 +155,11 @@ export function createPendingApproval(
|
||||
`INSERT OR IGNORE INTO pending_approvals
|
||||
(approval_id, session_id, request_id, action, payload, created_at,
|
||||
agent_group_id, channel_type, platform_id, platform_message_id, expires_at, status,
|
||||
title, options_json, approver_user_id)
|
||||
title, options_json, approver_user_id, eligibility, approver_scope, dedup_key)
|
||||
VALUES
|
||||
(@approval_id, @session_id, @request_id, @action, @payload, @created_at,
|
||||
@agent_group_id, @channel_type, @platform_id, @platform_message_id, @expires_at, @status,
|
||||
@title, @options_json, @approver_user_id)`,
|
||||
@title, @options_json, @approver_user_id, @eligibility, @approver_scope, @dedup_key)`,
|
||||
)
|
||||
.run({
|
||||
session_id: null,
|
||||
@@ -170,11 +170,21 @@ export function createPendingApproval(
|
||||
expires_at: null,
|
||||
status: 'pending',
|
||||
approver_user_id: null,
|
||||
eligibility: 'admins-of-scope',
|
||||
approver_scope: 'group',
|
||||
dedup_key: null,
|
||||
...pa,
|
||||
});
|
||||
return result.changes > 0;
|
||||
}
|
||||
|
||||
/** In-flight lookup for `requestApproval`'s dedup: any live row with this key blocks a repeat request. */
|
||||
export function getPendingApprovalByDedupKey(dedupKey: string): PendingApproval | undefined {
|
||||
return getDb().prepare('SELECT * FROM pending_approvals WHERE dedup_key = ? LIMIT 1').get(dedupKey) as
|
||||
| PendingApproval
|
||||
| undefined;
|
||||
}
|
||||
|
||||
export function getPendingApproval(approvalId: string): PendingApproval | undefined {
|
||||
return getDb().prepare('SELECT * FROM pending_approvals WHERE approval_id = ?').get(approvalId) as
|
||||
| PendingApproval
|
||||
@@ -230,8 +240,9 @@ export function getAskQuestionRender(
|
||||
| undefined;
|
||||
if (a?.title) return { title: a.title, options: JSON.parse(a.options_json) };
|
||||
|
||||
// Channel-registration + unknown-sender approvals persist title/options_json
|
||||
// the same way pending_approvals does — just SELECT and return.
|
||||
// Channel-registration approvals persist title/options_json the same way
|
||||
// pending_approvals does — just SELECT and return. (Unknown-sender approvals
|
||||
// are pending_approvals rows since the sender fold.)
|
||||
if (hasTable(getDb(), 'pending_channel_approvals')) {
|
||||
const c = getDb()
|
||||
.prepare('SELECT title, options_json FROM pending_channel_approvals WHERE messaging_group_id = ?')
|
||||
@@ -239,12 +250,5 @@ export function getAskQuestionRender(
|
||||
if (c?.title) return { title: c.title, options: JSON.parse(c.options_json) };
|
||||
}
|
||||
|
||||
if (hasTable(getDb(), 'pending_sender_approvals')) {
|
||||
const s = getDb().prepare('SELECT title, options_json FROM pending_sender_approvals WHERE id = ?').get(id) as
|
||||
| { title: string; options_json: string }
|
||||
| undefined;
|
||||
if (s?.title) return { title: s.title, options: JSON.parse(s.options_json) };
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -94,6 +94,10 @@ export async function handleCreateAgent(content: Record<string, unknown>, sessio
|
||||
* a confined (non-global) agent group. `session` is the requesting parent.
|
||||
*/
|
||||
export const applyCreateAgent: ApprovalHandler = async ({ session, payload, notify }) => {
|
||||
if (!session) {
|
||||
log.warn('create_agent approval resolved without a session — dropping');
|
||||
return;
|
||||
}
|
||||
const name = typeof payload.name === 'string' ? payload.name : '';
|
||||
const instructions = typeof payload.instructions === 'string' ? payload.instructions : null;
|
||||
|
||||
|
||||
@@ -4,6 +4,10 @@ import type { ApprovalHandler } from '../approvals/index.js';
|
||||
import { performAgentRoute, type RoutableAgentMessage } from './agent-route.js';
|
||||
|
||||
export const applyA2aMessageGate: ApprovalHandler = async ({ session, payload, notify }) => {
|
||||
if (!session) {
|
||||
log.warn('a2a_message_gate approval resolved without a session — dropping');
|
||||
return;
|
||||
}
|
||||
const { id, platform_id, content, in_reply_to } = payload;
|
||||
if (typeof platform_id !== 'string' || !platform_id) {
|
||||
notify('Message approved but the target agent group was missing from the request.');
|
||||
|
||||
@@ -100,7 +100,7 @@ describe('approval-resolved callbacks', () => {
|
||||
expect(events[0].outcome).toBe('reject');
|
||||
expect(events[0].approval.approval_id).toBe('appr-reject-1');
|
||||
expect(events[0].approval.action).toBe('test_reject_action');
|
||||
expect(events[0].session.id).toBe('sess-1');
|
||||
expect(events[0].session?.id).toBe('sess-1');
|
||||
expect(events[0].userId).toBe('slack:admin-1');
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,222 @@
|
||||
/**
|
||||
* mayResolve matrix — the one click-authorization rule for every hold.
|
||||
*
|
||||
* Covers each eligibility kind × clicker role × approver scope, including:
|
||||
* - exclusive named approvers (a2a policy semantics: nobody else, not even
|
||||
* an owner, may resolve)
|
||||
* - admins-of-scope with and without a delivered approver (the
|
||||
* sender/channel "named-or-admin" semantic)
|
||||
* - the null-anchor variant (owners + global admins only)
|
||||
* - the D1 fix: a 'global'-scope hold rejects a scoped admin's click even
|
||||
* though the eligibility rule would otherwise accept it
|
||||
*
|
||||
* Plus an end-to-end D1 regression through the real response handler: a
|
||||
* global-blast CLI hold (e.g. roles grant) clicked by a scoped admin is
|
||||
* ignored; the owner's click resolves it.
|
||||
*/
|
||||
import * as fs from 'fs';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { initTestDb, closeDb, runMigrations } from '../../db/index.js';
|
||||
import { createAgentGroup } from '../../db/agent-groups.js';
|
||||
import { createSession, createPendingApproval, getPendingApproval } from '../../db/sessions.js';
|
||||
import { upsertUser } from '../permissions/db/users.js';
|
||||
import { grantRole } from '../permissions/db/user-roles.js';
|
||||
import { initSessionFolder } from '../../session-manager.js';
|
||||
import { eligibilityOf, mayResolve } from './eligibility.js';
|
||||
import { registerApprovalHandler } from './primitive.js';
|
||||
import { handleApprovalsResponse } from './response-handler.js';
|
||||
|
||||
vi.mock('../../container-runner.js', () => ({
|
||||
wakeContainer: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock('../../config.js', async () => {
|
||||
const actual = await vi.importActual('../../config.js');
|
||||
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-eligibility' };
|
||||
});
|
||||
|
||||
const TEST_DIR = '/tmp/nanoclaw-test-eligibility';
|
||||
|
||||
function now() {
|
||||
return new Date().toISOString();
|
||||
}
|
||||
|
||||
const OWNER = 'slack:owner';
|
||||
const GLOBAL_ADMIN = 'slack:global-admin';
|
||||
const SCOPED_ADMIN = 'slack:scoped-admin'; // admin @ ag-1
|
||||
const OTHER_ADMIN = 'slack:other-admin'; // admin @ ag-2
|
||||
const DELIVEREE = 'slack:deliveree'; // no role — the user a card was delivered to
|
||||
const RANDO = 'slack:rando'; // no role
|
||||
|
||||
beforeEach(() => {
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DIR, { recursive: true });
|
||||
const db = initTestDb();
|
||||
runMigrations(db);
|
||||
|
||||
createAgentGroup({ id: 'ag-1', name: 'One', folder: 'one', agent_provider: null, created_at: now() });
|
||||
createAgentGroup({ id: 'ag-2', name: 'Two', folder: 'two', agent_provider: null, created_at: now() });
|
||||
|
||||
for (const id of [OWNER, GLOBAL_ADMIN, SCOPED_ADMIN, OTHER_ADMIN, DELIVEREE, RANDO]) {
|
||||
upsertUser({ id, kind: 'slack', display_name: id, created_at: now() });
|
||||
}
|
||||
grantRole({ user_id: OWNER, role: 'owner', agent_group_id: null, granted_by: null, granted_at: now() });
|
||||
grantRole({ user_id: GLOBAL_ADMIN, role: 'admin', agent_group_id: null, granted_by: null, granted_at: now() });
|
||||
grantRole({ user_id: SCOPED_ADMIN, role: 'admin', agent_group_id: 'ag-1', granted_by: null, granted_at: now() });
|
||||
grantRole({ user_id: OTHER_ADMIN, role: 'admin', agent_group_id: 'ag-2', granted_by: null, granted_at: now() });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
closeDb();
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('mayResolve matrix', () => {
|
||||
it('exclusive: only the named user, regardless of rank', () => {
|
||||
const e = { kind: 'exclusive', approverUserId: DELIVEREE } as const;
|
||||
expect(mayResolve(e, 'group', DELIVEREE)).toBe(true);
|
||||
expect(mayResolve(e, 'group', OWNER)).toBe(false);
|
||||
expect(mayResolve(e, 'group', GLOBAL_ADMIN)).toBe(false);
|
||||
expect(mayResolve(e, 'group', SCOPED_ADMIN)).toBe(false);
|
||||
expect(mayResolve(e, 'group', RANDO)).toBe(false);
|
||||
expect(mayResolve(e, 'group', null)).toBe(false);
|
||||
});
|
||||
|
||||
it('exclusive ∩ global scope: the named user must also be owner/global admin', () => {
|
||||
expect(mayResolve({ kind: 'exclusive', approverUserId: DELIVEREE }, 'global', DELIVEREE)).toBe(false);
|
||||
expect(mayResolve({ kind: 'exclusive', approverUserId: OWNER }, 'global', OWNER)).toBe(true);
|
||||
expect(mayResolve({ kind: 'exclusive', approverUserId: GLOBAL_ADMIN }, 'global', GLOBAL_ADMIN)).toBe(true);
|
||||
});
|
||||
|
||||
it('admins-of-scope(group) with a delivered approver: named-or-admin', () => {
|
||||
const e = { kind: 'admins-of-scope', agentGroupId: 'ag-1', deliveredTo: DELIVEREE } as const;
|
||||
expect(mayResolve(e, 'group', DELIVEREE)).toBe(true); // delivered-to shortcut
|
||||
expect(mayResolve(e, 'group', SCOPED_ADMIN)).toBe(true);
|
||||
expect(mayResolve(e, 'group', GLOBAL_ADMIN)).toBe(true);
|
||||
expect(mayResolve(e, 'group', OWNER)).toBe(true);
|
||||
expect(mayResolve(e, 'group', OTHER_ADMIN)).toBe(false); // admin of another group
|
||||
expect(mayResolve(e, 'group', RANDO)).toBe(false);
|
||||
expect(mayResolve(e, 'group', null)).toBe(false);
|
||||
});
|
||||
|
||||
it('admins-of-scope(group) without a delivered approver: pure admin chain', () => {
|
||||
const e = { kind: 'admins-of-scope', agentGroupId: 'ag-1', deliveredTo: null } as const;
|
||||
expect(mayResolve(e, 'group', SCOPED_ADMIN)).toBe(true);
|
||||
expect(mayResolve(e, 'group', GLOBAL_ADMIN)).toBe(true);
|
||||
expect(mayResolve(e, 'group', OWNER)).toBe(true);
|
||||
expect(mayResolve(e, 'group', DELIVEREE)).toBe(false);
|
||||
expect(mayResolve(e, 'group', OTHER_ADMIN)).toBe(false);
|
||||
});
|
||||
|
||||
it('admins-of-scope(null): owners and global admins only', () => {
|
||||
const e = { kind: 'admins-of-scope', agentGroupId: null, deliveredTo: null } as const;
|
||||
expect(mayResolve(e, 'group', OWNER)).toBe(true);
|
||||
expect(mayResolve(e, 'group', GLOBAL_ADMIN)).toBe(true);
|
||||
expect(mayResolve(e, 'group', SCOPED_ADMIN)).toBe(false);
|
||||
expect(mayResolve(e, 'group', RANDO)).toBe(false);
|
||||
});
|
||||
|
||||
it('admins-of-scope(null) with a delivered approver keeps the delivered-to shortcut (channel semantics)', () => {
|
||||
const e = { kind: 'admins-of-scope', agentGroupId: null, deliveredTo: DELIVEREE } as const;
|
||||
expect(mayResolve(e, 'group', DELIVEREE)).toBe(true);
|
||||
expect(mayResolve(e, 'group', SCOPED_ADMIN)).toBe(false);
|
||||
});
|
||||
|
||||
it('D1 overlay: global scope rejects everyone below owner/global admin', () => {
|
||||
const e = { kind: 'admins-of-scope', agentGroupId: 'ag-1', deliveredTo: DELIVEREE } as const;
|
||||
expect(mayResolve(e, 'global', SCOPED_ADMIN)).toBe(false); // the D1 exploit, closed
|
||||
expect(mayResolve(e, 'global', DELIVEREE)).toBe(false);
|
||||
expect(mayResolve(e, 'global', OTHER_ADMIN)).toBe(false);
|
||||
expect(mayResolve(e, 'global', OWNER)).toBe(true);
|
||||
expect(mayResolve(e, 'global', GLOBAL_ADMIN)).toBe(true);
|
||||
});
|
||||
|
||||
it('eligibilityOf maps row columns onto the rule', () => {
|
||||
const base = { agent_group_id: 'ag-1' };
|
||||
expect(eligibilityOf({ ...base, eligibility: 'exclusive', approver_user_id: DELIVEREE })).toEqual({
|
||||
kind: 'exclusive',
|
||||
approverUserId: DELIVEREE,
|
||||
});
|
||||
expect(eligibilityOf({ ...base, eligibility: 'admins-of-scope', approver_user_id: DELIVEREE })).toEqual({
|
||||
kind: 'admins-of-scope',
|
||||
agentGroupId: 'ag-1',
|
||||
deliveredTo: DELIVEREE,
|
||||
});
|
||||
expect(eligibilityOf({ ...base, eligibility: 'admins-of-scope', approver_user_id: null })).toEqual({
|
||||
kind: 'admins-of-scope',
|
||||
agentGroupId: 'ag-1',
|
||||
deliveredTo: null,
|
||||
});
|
||||
// Malformed exclusive (no named user) falls back to the admin chain
|
||||
// instead of bricking the hold.
|
||||
expect(eligibilityOf({ ...base, eligibility: 'exclusive', approver_user_id: null })).toEqual({
|
||||
kind: 'admins-of-scope',
|
||||
agentGroupId: 'ag-1',
|
||||
deliveredTo: null,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('D1 regression — global-blast hold through the real response handler', () => {
|
||||
beforeEach(() => {
|
||||
createSession({
|
||||
id: 'sess-1',
|
||||
agent_group_id: 'ag-1',
|
||||
messaging_group_id: null,
|
||||
thread_id: null,
|
||||
agent_provider: null,
|
||||
status: 'active',
|
||||
container_status: 'stopped',
|
||||
last_active: now(),
|
||||
created_at: now(),
|
||||
});
|
||||
initSessionFolder('ag-1', 'sess-1');
|
||||
});
|
||||
|
||||
it("a scoped admin's click on a roles-grant-style hold is ignored; the owner's click resolves it", async () => {
|
||||
const applied: string[] = [];
|
||||
registerApprovalHandler('test_global_blast', async ({ userId }) => {
|
||||
applied.push(userId);
|
||||
});
|
||||
|
||||
createPendingApproval({
|
||||
approval_id: 'appr-global-1',
|
||||
session_id: 'sess-1',
|
||||
request_id: 'appr-global-1',
|
||||
action: 'test_global_blast',
|
||||
payload: JSON.stringify({}),
|
||||
created_at: now(),
|
||||
agent_group_id: 'ag-1',
|
||||
title: 'CLI: roles-grant',
|
||||
options_json: JSON.stringify([]),
|
||||
approver_scope: 'global',
|
||||
});
|
||||
|
||||
// Scoped admin of the requesting group clicks approve — pre-D1 this
|
||||
// resolved a global privilege grant; now it is ignored and the hold stays.
|
||||
const claimedByScoped = await handleApprovalsResponse({
|
||||
questionId: 'appr-global-1',
|
||||
value: 'approve',
|
||||
userId: 'scoped-admin',
|
||||
channelType: 'slack',
|
||||
platformId: 'dm-scoped',
|
||||
threadId: null,
|
||||
});
|
||||
expect(claimedByScoped).toBe(true);
|
||||
expect(applied).toEqual([]);
|
||||
expect(getPendingApproval('appr-global-1')).toBeDefined();
|
||||
|
||||
// The owner's click resolves it.
|
||||
await handleApprovalsResponse({
|
||||
questionId: 'appr-global-1',
|
||||
value: 'approve',
|
||||
userId: 'owner',
|
||||
channelType: 'slack',
|
||||
platformId: 'dm-owner',
|
||||
threadId: null,
|
||||
});
|
||||
expect(applied).toEqual([OWNER]);
|
||||
expect(getPendingApproval('appr-global-1')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,68 @@
|
||||
/**
|
||||
* Approver eligibility — the one click-authorization rule for every hold.
|
||||
*
|
||||
* The hold-record contract (guarded-actions phase 1) is carried on the
|
||||
* existing tables: a hold has an id, an action, a payload, an eligibility
|
||||
* rule (who may resolve it), an approver scope (the action's blast radius),
|
||||
* a restart policy, and an optional expiry. On `pending_approvals` these map
|
||||
* to `approval_id` / `action` / `payload` / (`eligibility` +
|
||||
* `approver_user_id` + `agent_group_id`) / `approver_scope` / `expires_at`;
|
||||
* the restart policy is derived from the action (`onecli_credential` rows are
|
||||
* swept-and-denied on boot, everything else is durable and keeps waiting).
|
||||
* `pending_channel_approvals` maps through a synthesized view
|
||||
* (channel-approval.ts) — the channel flow keeps its own table.
|
||||
*
|
||||
* Two eligibility kinds:
|
||||
* - `exclusive` — only the named user may resolve (an a2a message policy's
|
||||
* approver). Nobody else, including owners.
|
||||
* - `admins-of-scope` — the admin chain of the anchoring agent group
|
||||
* (scoped admin / global admin / owner), or owners + global admins when
|
||||
* the anchor is null. When the hold records the user the card was
|
||||
* delivered to, that user may also resolve — the sender/channel
|
||||
* "named-or-admin" semantic, preserved verbatim from the pre-fold tables.
|
||||
*
|
||||
* The approver-scope overlay is the D1 fix: a hold whose action has global
|
||||
* blast radius (e.g. `roles grant`) can only be resolved by an owner or
|
||||
* global admin — a scoped admin's click is rejected regardless of the
|
||||
* eligibility rule.
|
||||
*
|
||||
* `mayResolve` replaces the three divergent click-auth copies (approvals
|
||||
* response handler, sender handler, channel handler) with one function.
|
||||
*/
|
||||
import type { ApproverEligibility, ApproverScope, PendingApproval } from '../../types.js';
|
||||
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
|
||||
|
||||
export type { ApproverEligibility, ApproverScope } from '../../types.js';
|
||||
|
||||
/** May `clickerUserId` (namespaced `<channel>:<handle>`) resolve a hold with this eligibility + scope? */
|
||||
export function mayResolve(e: ApproverEligibility, scope: ApproverScope, clickerUserId: string | null): boolean {
|
||||
if (!clickerUserId) return false;
|
||||
|
||||
const globalScopeOk = scope !== 'global' || isOwner(clickerUserId) || isGlobalAdmin(clickerUserId);
|
||||
|
||||
if (e.kind === 'exclusive') {
|
||||
return clickerUserId === e.approverUserId && globalScopeOk;
|
||||
}
|
||||
|
||||
const eligible =
|
||||
(e.deliveredTo !== null && clickerUserId === e.deliveredTo) ||
|
||||
(e.agentGroupId
|
||||
? hasAdminPrivilege(clickerUserId, e.agentGroupId)
|
||||
: isOwner(clickerUserId) || isGlobalAdmin(clickerUserId));
|
||||
|
||||
return eligible && globalScopeOk;
|
||||
}
|
||||
|
||||
/** The eligibility rule a `pending_approvals` row encodes. */
|
||||
export function eligibilityOf(
|
||||
approval: Pick<PendingApproval, 'eligibility' | 'approver_user_id' | 'agent_group_id'>,
|
||||
): ApproverEligibility {
|
||||
if (approval.eligibility === 'exclusive' && approval.approver_user_id) {
|
||||
return { kind: 'exclusive', approverUserId: approval.approver_user_id };
|
||||
}
|
||||
return {
|
||||
kind: 'admins-of-scope',
|
||||
agentGroupId: approval.agent_group_id,
|
||||
deliveredTo: approval.eligibility === 'exclusive' ? null : approval.approver_user_id,
|
||||
};
|
||||
}
|
||||
@@ -25,26 +25,31 @@ import { notifyApprovalResolved } from './primitive.js';
|
||||
* attribution — the why, not the who (the rejecting admin may belong to a
|
||||
* different owner than the requesting agent). Callers are responsible for
|
||||
* clamping the reason length before passing it in.
|
||||
*
|
||||
* `session` is null for sessionless holds (e.g. sender admission) — there is
|
||||
* no agent to notify or wake, so only the row delete + resolved callbacks run.
|
||||
*/
|
||||
export async function finalizeReject(
|
||||
approval: PendingApproval,
|
||||
session: Session,
|
||||
session: Session | null,
|
||||
userId: string,
|
||||
reason?: string,
|
||||
): Promise<void> {
|
||||
const text = reason
|
||||
? `Your ${approval.action} request was rejected by admin: "${reason}"`
|
||||
: `Your ${approval.action} request was rejected by admin.`;
|
||||
if (session) {
|
||||
const text = reason
|
||||
? `Your ${approval.action} request was rejected by admin: "${reason}"`
|
||||
: `Your ${approval.action} request was rejected by admin.`;
|
||||
|
||||
writeSessionMessage(session.agent_group_id, session.id, {
|
||||
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
kind: 'chat',
|
||||
timestamp: new Date().toISOString(),
|
||||
platformId: session.agent_group_id,
|
||||
channelType: 'agent',
|
||||
threadId: null,
|
||||
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
|
||||
});
|
||||
writeSessionMessage(session.agent_group_id, session.id, {
|
||||
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
kind: 'chat',
|
||||
timestamp: new Date().toISOString(),
|
||||
platformId: session.agent_group_id,
|
||||
channelType: 'agent',
|
||||
threadId: null,
|
||||
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
|
||||
});
|
||||
}
|
||||
|
||||
log.info('Approval rejected', {
|
||||
approvalId: approval.approval_id,
|
||||
@@ -55,5 +60,5 @@ export async function finalizeReject(
|
||||
|
||||
deletePendingApproval(approval.approval_id);
|
||||
await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
|
||||
await wakeContainer(session);
|
||||
if (session) await wakeContainer(session);
|
||||
}
|
||||
|
||||
@@ -19,12 +19,13 @@
|
||||
*/
|
||||
import { OneCLI, type ApprovalRequest, type ManualApprovalHandle } from '@onecli-sh/sdk';
|
||||
|
||||
import { pickApprovalDelivery, pickApprover } from './primitive.js';
|
||||
import { notifyApprovalResolved, pickApprovalDelivery, pickApprover } from './primitive.js';
|
||||
import { ONECLI_API_KEY, ONECLI_URL } from '../../config.js';
|
||||
import { getAgentGroup } from '../../db/agent-groups.js';
|
||||
import {
|
||||
createPendingApproval,
|
||||
deletePendingApproval,
|
||||
getPendingApproval,
|
||||
getPendingApprovalsByAction,
|
||||
updatePendingApprovalStatus,
|
||||
} from '../../db/sessions.js';
|
||||
@@ -64,20 +65,29 @@ function shortApprovalId(): string {
|
||||
return `oa-${Math.random().toString(36).slice(2, 10)}`;
|
||||
}
|
||||
|
||||
/** Called from the approvals response handler when a card button is clicked. */
|
||||
export function resolveOneCLIApproval(approvalId: string, selectedOption: string): boolean {
|
||||
/** Called from the approvals response handler when a card button is clicked. `resolvedBy` = namespaced clicker id. */
|
||||
export function resolveOneCLIApproval(approvalId: string, selectedOption: string, resolvedBy = ''): boolean {
|
||||
const state = pending.get(approvalId);
|
||||
if (!state) return false;
|
||||
pending.delete(approvalId);
|
||||
clearTimeout(state.timer);
|
||||
|
||||
const decision: Decision = selectedOption === 'approve' ? 'approve' : 'deny';
|
||||
const row = getPendingApproval(approvalId);
|
||||
updatePendingApprovalStatus(approvalId, decision === 'approve' ? 'approved' : 'rejected');
|
||||
// Card is auto-edited to "✅ <option>" by chat-sdk-bridge's onAction handler,
|
||||
// so we don't need to deliver an edit here.
|
||||
deletePendingApproval(approvalId);
|
||||
|
||||
state.resolve(decision);
|
||||
if (row) {
|
||||
void notifyApprovalResolved({
|
||||
approval: row,
|
||||
session: null,
|
||||
outcome: decision === 'approve' ? 'approve' : 'reject',
|
||||
userId: resolvedBy,
|
||||
});
|
||||
}
|
||||
log.info('OneCLI approval resolved', { approvalId, decision });
|
||||
return true;
|
||||
}
|
||||
@@ -222,6 +232,7 @@ async function expireApproval(approvalId: string, reason: string): Promise<void>
|
||||
updatePendingApprovalStatus(approvalId, 'expired');
|
||||
await editCardExpired(row, reason);
|
||||
deletePendingApproval(approvalId);
|
||||
await notifyApprovalResolved({ approval: row, session: null, outcome: 'expire', userId: '' });
|
||||
log.info('OneCLI approval expired', { approvalId, reason });
|
||||
}
|
||||
|
||||
@@ -251,6 +262,7 @@ async function sweepStaleApprovals(): Promise<void> {
|
||||
for (const row of rows) {
|
||||
await editCardExpired(row, 'host restarted');
|
||||
deletePendingApproval(row.approval_id);
|
||||
await notifyApprovalResolved({ approval: row, session: null, outcome: 'sweep', userId: '' });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
*/
|
||||
import { normalizeOptions, type RawOption } from '../../channels/ask-question.js';
|
||||
import { getMessagingGroup } from '../../db/messaging-groups.js';
|
||||
import { createPendingApproval, getSession } from '../../db/sessions.js';
|
||||
import { createPendingApproval, getPendingApprovalByDedupKey, getSession } from '../../db/sessions.js';
|
||||
import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { wakeContainer } from '../../container-runner.js';
|
||||
import { log } from '../../log.js';
|
||||
@@ -57,11 +57,12 @@ const APPROVAL_OPTIONS: RawOption[] = [
|
||||
// their `requestApproval()` calls.
|
||||
|
||||
export interface ApprovalHandlerContext {
|
||||
session: Session;
|
||||
/** Requesting agent's session. Null for sessionless holds (e.g. sender admission). */
|
||||
session: Session | null;
|
||||
payload: Record<string, unknown>;
|
||||
/** User ID of the admin who approved. Empty string if unknown. */
|
||||
userId: string;
|
||||
/** Send a system chat message to the requesting agent's session. */
|
||||
/** Send a system chat message to the requesting agent's session. No-op when sessionless. */
|
||||
notify: (text: string) => void;
|
||||
}
|
||||
|
||||
@@ -88,14 +89,20 @@ export function getApprovalHandler(action: string): ApprovalHandler | undefined
|
||||
// out. Callback errors are logged and isolated; they never block resolution.
|
||||
//
|
||||
// Only authorized clicks resolve an approval (the response handler's
|
||||
// isAuthorizedApprovalClick gate runs first), so callbacks never fire for
|
||||
// unauthorized responses.
|
||||
// mayResolve gate runs first), so callbacks never fire for unauthorized
|
||||
// responses. Non-click resolutions (OneCLI expiry timers, the boot sweep)
|
||||
// announce here too, with outcome 'expire' / 'sweep'.
|
||||
|
||||
export interface ApprovalResolvedEvent {
|
||||
/**
|
||||
* The resolved hold. For holds that live outside pending_approvals
|
||||
* (channel registration) this is a synthesized view of the same shape.
|
||||
*/
|
||||
approval: PendingApproval;
|
||||
session: Session;
|
||||
outcome: 'approve' | 'reject';
|
||||
/** Namespaced user ID (`<channel>:<handle>`) of the resolving admin. Empty string if unknown. */
|
||||
/** Requesting agent's session; null for sessionless holds (sender admission, OneCLI, channel registration). */
|
||||
session: Session | null;
|
||||
outcome: 'approve' | 'reject' | 'expire' | 'sweep';
|
||||
/** Namespaced user ID (`<channel>:<handle>`) of the resolving admin. Empty string if unknown (expiry/sweep). */
|
||||
userId: string;
|
||||
}
|
||||
|
||||
@@ -200,7 +207,14 @@ export function notifyAgent(session: Session, text: string): void {
|
||||
}
|
||||
|
||||
export interface RequestApprovalOptions {
|
||||
session: Session;
|
||||
/**
|
||||
* Requesting agent's session. Omit for sessionless holds (e.g. sender
|
||||
* admission) — failure notices are then logged instead of chat-relayed,
|
||||
* and the hold anchors to `agentGroupId`.
|
||||
*/
|
||||
session?: Session;
|
||||
/** Eligibility anchor when there is no session. Ignored when `session` is set (its agent group wins). */
|
||||
agentGroupId?: string;
|
||||
agentName: string;
|
||||
/** Free-form action identifier. Must match the key the consumer registered via registerApprovalHandler. */
|
||||
action: string;
|
||||
@@ -210,8 +224,32 @@ export interface RequestApprovalOptions {
|
||||
title: string;
|
||||
/** Card body shown to the admin. */
|
||||
question: string;
|
||||
/** Deliver the card to this specific user instead of all of the session group's admins. */
|
||||
/**
|
||||
* Deliver the card to this specific user AND make the hold exclusively
|
||||
* theirs to resolve (eligibility 'exclusive' — an a2a policy's approver).
|
||||
*/
|
||||
approverUserId?: string;
|
||||
/**
|
||||
* The action's blast radius. 'global' holds (privilege grants, cross-group
|
||||
* writes) can only be resolved by an owner or global admin. Default 'group'.
|
||||
*/
|
||||
approverScope?: 'group' | 'global';
|
||||
/** Card buttons. Default: Approve / Reject / Reject with reason…. */
|
||||
options?: RawOption[];
|
||||
/**
|
||||
* In-flight dedup: while a pending row carries this key, a repeat request
|
||||
* with the same key is dropped without a second card.
|
||||
*/
|
||||
dedupKey?: string;
|
||||
/**
|
||||
* Record the user the card is delivered to on the hold, letting them
|
||||
* resolve it alongside the scope's admins even if their role changes
|
||||
* mid-flight (the sender/channel "named-or-admin" semantic). Off by
|
||||
* default — module holds authorize purely by the admin chain.
|
||||
*/
|
||||
recordDeliveredApprover?: boolean;
|
||||
/** Channel preference for the approver-DM walk when there is no session to derive it from. */
|
||||
originChannelType?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -221,36 +259,52 @@ export interface RequestApprovalOptions {
|
||||
* approval handler for this action via the response dispatcher.
|
||||
*/
|
||||
export async function requestApproval(opts: RequestApprovalOptions): Promise<void> {
|
||||
const { session, action, payload, title, question, agentName, approverUserId } = opts;
|
||||
const { session, action, payload, title, question, agentName, approverUserId, dedupKey } = opts;
|
||||
|
||||
const approvers = approverUserId ? [approverUserId] : pickApprover(session.agent_group_id);
|
||||
if (approvers.length === 0) {
|
||||
notifyAgent(session, `${action} failed: no owner or admin configured to approve.`);
|
||||
const agentGroupId = session?.agent_group_id ?? opts.agentGroupId ?? null;
|
||||
|
||||
const fail = (text: string): void => {
|
||||
if (session) notifyAgent(session, `${action} failed: ${text}`);
|
||||
else log.warn('Approval request failed', { action, agentGroupId, reason: text });
|
||||
};
|
||||
|
||||
if (dedupKey && getPendingApprovalByDedupKey(dedupKey)) {
|
||||
log.debug('Approval request already in flight — dropping duplicate', { action, dedupKey });
|
||||
return;
|
||||
}
|
||||
|
||||
const originChannelType = session.messaging_group_id
|
||||
? (getMessagingGroup(session.messaging_group_id)?.channel_type ?? '')
|
||||
: '';
|
||||
const approvers = approverUserId ? [approverUserId] : pickApprover(agentGroupId);
|
||||
if (approvers.length === 0) {
|
||||
fail('no owner or admin configured to approve.');
|
||||
return;
|
||||
}
|
||||
|
||||
const originChannelType =
|
||||
opts.originChannelType ??
|
||||
(session?.messaging_group_id ? (getMessagingGroup(session.messaging_group_id)?.channel_type ?? '') : '');
|
||||
|
||||
const target = await pickApprovalDelivery(approvers, originChannelType);
|
||||
if (!target) {
|
||||
notifyAgent(session, `${action} failed: no DM channel found for any eligible approver.`);
|
||||
fail('no DM channel found for any eligible approver.');
|
||||
return;
|
||||
}
|
||||
|
||||
const approvalId = `appr-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
const normalizedOptions = normalizeOptions(APPROVAL_OPTIONS);
|
||||
const cardOptions = opts.options ?? APPROVAL_OPTIONS;
|
||||
createPendingApproval({
|
||||
approval_id: approvalId,
|
||||
session_id: session.id,
|
||||
session_id: session?.id ?? null,
|
||||
request_id: approvalId,
|
||||
action,
|
||||
payload: JSON.stringify(payload),
|
||||
created_at: new Date().toISOString(),
|
||||
agent_group_id: agentGroupId,
|
||||
title,
|
||||
options_json: JSON.stringify(normalizedOptions),
|
||||
approver_user_id: approverUserId ?? null,
|
||||
options_json: JSON.stringify(normalizeOptions(cardOptions)),
|
||||
approver_user_id: approverUserId ?? (opts.recordDeliveredApprover ? target.userId : null),
|
||||
eligibility: approverUserId ? 'exclusive' : 'admins-of-scope',
|
||||
approver_scope: opts.approverScope ?? 'group',
|
||||
dedup_key: dedupKey ?? null,
|
||||
});
|
||||
|
||||
const adapter = getDeliveryAdapter();
|
||||
@@ -266,12 +320,12 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise<voi
|
||||
questionId: approvalId,
|
||||
title,
|
||||
question,
|
||||
options: APPROVAL_OPTIONS,
|
||||
options: cardOptions,
|
||||
}),
|
||||
);
|
||||
} catch (err) {
|
||||
log.error('Failed to deliver approval card', { action, approvalId, err });
|
||||
notifyAgent(session, `${action} failed: could not deliver approval request to ${target.userId}.`);
|
||||
fail(`could not deliver approval request to ${target.userId}.`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,9 @@
|
||||
* 2. OneCLI credential approvals (`action = 'onecli_credential'`). Resolved
|
||||
* via an in-memory Promise — see onecli-approvals.ts.
|
||||
*
|
||||
* Click authorization is `mayResolve` over the hold's eligibility rule +
|
||||
* approver scope (eligibility.ts) — the one shared rule for every hold.
|
||||
*
|
||||
* The response handler is registered via core's `registerResponseHandler`;
|
||||
* core iterates handlers and the first one to return `true` claims the response.
|
||||
*/
|
||||
@@ -20,8 +23,8 @@ import { deletePendingApproval, getPendingApproval, getSession } from '../../db/
|
||||
import type { ResponsePayload } from '../../response-registry.js';
|
||||
import { log } from '../../log.js';
|
||||
import { writeSessionMessage } from '../../session-manager.js';
|
||||
import type { PendingApproval } from '../../types.js';
|
||||
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
|
||||
import type { PendingApproval, Session } from '../../types.js';
|
||||
import { eligibilityOf, mayResolve } from './eligibility.js';
|
||||
import { finalizeReject } from './finalize.js';
|
||||
import { ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
|
||||
import { getApprovalHandler, notifyApprovalResolved, REJECT_WITH_REASON_VALUE } from './primitive.js';
|
||||
@@ -31,7 +34,8 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
|
||||
const approval = getPendingApproval(payload.questionId);
|
||||
if (!approval) return false;
|
||||
|
||||
if (!isAuthorizedApprovalClick(approval, payload)) {
|
||||
const clickerId = namespacedUserId(payload);
|
||||
if (!mayResolve(eligibilityOf(approval), approval.approver_scope, clickerId)) {
|
||||
log.warn('Ignoring unauthorized approval response', {
|
||||
approvalId: approval.approval_id,
|
||||
action: approval.action,
|
||||
@@ -42,7 +46,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
|
||||
}
|
||||
|
||||
if (approval.action === ONECLI_ACTION) {
|
||||
if (resolveOneCLIApproval(payload.questionId, payload.value)) {
|
||||
if (resolveOneCLIApproval(payload.questionId, payload.value, clickerId ?? '')) {
|
||||
return true;
|
||||
}
|
||||
// Row exists but the in-memory resolver is gone (timer fired or the process
|
||||
@@ -51,7 +55,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
|
||||
return true;
|
||||
}
|
||||
|
||||
await handleRegisteredApproval(approval, payload.value, namespacedUserId(payload) ?? '');
|
||||
await handleRegisteredApproval(approval, payload.value, clickerId ?? '');
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -60,12 +64,11 @@ async function handleRegisteredApproval(
|
||||
selectedOption: string,
|
||||
userId: string,
|
||||
): Promise<void> {
|
||||
if (!approval.session_id) {
|
||||
deletePendingApproval(approval.approval_id);
|
||||
return;
|
||||
}
|
||||
const session = getSession(approval.session_id);
|
||||
if (!session) {
|
||||
// Sessionless holds (sender admission) carry session_id null and resolve
|
||||
// without an agent to notify; a session-BOUND hold whose session vanished
|
||||
// is stale — drop it.
|
||||
const session: Session | null = approval.session_id ? (getSession(approval.session_id) ?? null) : null;
|
||||
if (approval.session_id && !session) {
|
||||
deletePendingApproval(approval.approval_id);
|
||||
return;
|
||||
}
|
||||
@@ -73,8 +76,10 @@ async function handleRegisteredApproval(
|
||||
// "Reject with reason…" — hold the row and capture the admin's next DM
|
||||
// instead of finalizing now. The agent is notified exactly once: after the
|
||||
// reason arrives, or after the sweep's timeout if the admin ghosts.
|
||||
// Sessionless holds have nobody to relay a reason to — plain reject.
|
||||
if (selectedOption === REJECT_WITH_REASON_VALUE) {
|
||||
await armReasonCapture(approval, session, userId);
|
||||
if (session) await armReasonCapture(approval, session, userId);
|
||||
else await finalizeReject(approval, null, userId);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -85,17 +90,19 @@ async function handleRegisteredApproval(
|
||||
}
|
||||
|
||||
// Approved — dispatch to the module that registered for this action.
|
||||
const notify = (text: string): void => {
|
||||
writeSessionMessage(session.agent_group_id, session.id, {
|
||||
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
kind: 'chat',
|
||||
timestamp: new Date().toISOString(),
|
||||
platformId: session.agent_group_id,
|
||||
channelType: 'agent',
|
||||
threadId: null,
|
||||
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
|
||||
});
|
||||
};
|
||||
const notify = session
|
||||
? (text: string): void => {
|
||||
writeSessionMessage(session.agent_group_id, session.id, {
|
||||
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
kind: 'chat',
|
||||
timestamp: new Date().toISOString(),
|
||||
platformId: session.agent_group_id,
|
||||
channelType: 'agent',
|
||||
threadId: null,
|
||||
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
|
||||
});
|
||||
}
|
||||
: (): void => {};
|
||||
|
||||
const handler = getApprovalHandler(approval.action);
|
||||
if (!handler) {
|
||||
@@ -106,7 +113,7 @@ async function handleRegisteredApproval(
|
||||
notify(`Your ${approval.action} was approved, but no handler is installed to apply it.`);
|
||||
deletePendingApproval(approval.approval_id);
|
||||
await notifyApprovalResolved({ approval, session, outcome: 'approve', userId });
|
||||
await wakeContainer(session);
|
||||
if (session) await wakeContainer(session);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -123,29 +130,10 @@ async function handleRegisteredApproval(
|
||||
|
||||
deletePendingApproval(approval.approval_id);
|
||||
await notifyApprovalResolved({ approval, session, outcome: 'approve', userId });
|
||||
await wakeContainer(session);
|
||||
if (session) await wakeContainer(session);
|
||||
}
|
||||
|
||||
function namespacedUserId(payload: ResponsePayload): string | null {
|
||||
if (!payload.userId) return null;
|
||||
return payload.userId.includes(':') ? payload.userId : `${payload.channelType}:${payload.userId}`;
|
||||
}
|
||||
|
||||
function isAuthorizedApprovalClick(approval: PendingApproval, payload: ResponsePayload): boolean {
|
||||
const userId = namespacedUserId(payload);
|
||||
if (!userId) return false;
|
||||
|
||||
// An approval may name a specific approver; only that exact user may resolve it.
|
||||
if (approval.approver_user_id) {
|
||||
return userId === approval.approver_user_id;
|
||||
}
|
||||
|
||||
const agentGroupId =
|
||||
approval.agent_group_id ?? (approval.session_id ? getSession(approval.session_id)?.agent_group_id : null);
|
||||
|
||||
if (!agentGroupId) {
|
||||
return isOwner(userId) || isGlobalAdmin(userId);
|
||||
}
|
||||
|
||||
return hasAdminPrivilege(userId, agentGroupId);
|
||||
}
|
||||
|
||||
@@ -358,7 +358,7 @@ describe('unknown-channel registration flow', () => {
|
||||
expect(stillPending).toBe(1);
|
||||
});
|
||||
|
||||
it('does not let a scoped admin connect an unknown channel to another agent group', async () => {
|
||||
it('does not let a scoped admin drive channel registration at all (D4: owner/global-admin eligibility)', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
const { getResponseHandlers } = await import('../../response-registry.js');
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
@@ -395,39 +395,28 @@ describe('unknown-channel registration flow', () => {
|
||||
messaging_group_id: string;
|
||||
};
|
||||
expect(pending).toBeDefined();
|
||||
// Registration creates groups/wirings — the card goes to the global chain
|
||||
// (the owner's DM), never to a scoped admin, even though one exists.
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
expect(deliverMock.mock.calls[0][1]).toBe('dm-scoped-admin');
|
||||
expect(deliverMock.mock.calls[0][1]).toBe('dm-owner');
|
||||
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.messaging_group_id,
|
||||
value: 'choose_existing',
|
||||
userId: 'scoped-admin',
|
||||
channelType: 'telegram',
|
||||
platformId: 'dm-scoped-admin',
|
||||
threadId: null,
|
||||
});
|
||||
if (claimed) break;
|
||||
}
|
||||
|
||||
const followupPayload = JSON.parse(deliverMock.mock.calls[1][4] as string) as {
|
||||
options: Array<{ label: string; value: string }>;
|
||||
};
|
||||
expect(followupPayload.options.map((option) => option.value)).toContain('connect:ag-1');
|
||||
expect(followupPayload.options.map((option) => option.value)).not.toContain('connect:ag-2');
|
||||
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.messaging_group_id,
|
||||
value: 'connect:ag-2',
|
||||
userId: 'scoped-admin',
|
||||
channelType: 'telegram',
|
||||
platformId: 'dm-scoped-admin',
|
||||
threadId: null,
|
||||
});
|
||||
if (claimed) break;
|
||||
// A scoped admin's clicks are ignored outright — no follow-up card, no
|
||||
// wiring, and the pending row stays for a real approver.
|
||||
for (const value of ['choose_existing', 'connect:ag-2', `connect:ag-1`]) {
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.messaging_group_id,
|
||||
value,
|
||||
userId: 'scoped-admin',
|
||||
channelType: 'telegram',
|
||||
platformId: 'dm-scoped-admin',
|
||||
threadId: null,
|
||||
});
|
||||
if (claimed) break;
|
||||
}
|
||||
}
|
||||
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1); // no follow-up card was sent
|
||||
const mgaCount = (
|
||||
getDb()
|
||||
.prepare('SELECT COUNT(*) AS c FROM messaging_group_agents WHERE messaging_group_id = ?')
|
||||
|
||||
@@ -52,9 +52,13 @@ import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { initGroupFilesystem } from '../../group-init.js';
|
||||
import { log } from '../../log.js';
|
||||
import type { InboundEvent } from '../../channels/adapter.js';
|
||||
import type { AgentGroup } from '../../types.js';
|
||||
import type { AgentGroup, PendingApproval } from '../../types.js';
|
||||
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
|
||||
import { createPendingChannelApproval, hasInFlightChannelApproval } from './db/pending-channel-approvals.js';
|
||||
import {
|
||||
createPendingChannelApproval,
|
||||
hasInFlightChannelApproval,
|
||||
type PendingChannelApproval,
|
||||
} from './db/pending-channel-approvals.js';
|
||||
import { hasAdminPrivilege } from './db/user-roles.js';
|
||||
|
||||
// ── Value constants (response handler in index.ts parses these) ──
|
||||
@@ -152,15 +156,14 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
||||
});
|
||||
return;
|
||||
}
|
||||
// Use first agent group for approver resolution — owners and global admins
|
||||
// are returned regardless of which group we pass.
|
||||
const referenceGroup = agentGroups[0];
|
||||
|
||||
const approvers = pickApprover(referenceGroup.id);
|
||||
// Registration creates groups and wirings — global blast radius, so the
|
||||
// approver comes from the global chain (global admins → owners), never from
|
||||
// whichever agent group happens to sort first.
|
||||
const approvers = pickApprover(null);
|
||||
if (approvers.length === 0) {
|
||||
log.warn('Channel registration skipped — no owner or admin configured', {
|
||||
log.warn('Channel registration skipped — no owner or global admin configured', {
|
||||
messagingGroupId,
|
||||
targetAgentGroupId: referenceGroup.id,
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -188,7 +191,6 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
||||
if (!delivery) {
|
||||
log.warn('Channel registration skipped — no DM channel for any approver', {
|
||||
messagingGroupId,
|
||||
targetAgentGroupId: referenceGroup.id,
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -208,9 +210,11 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
|
||||
const question = buildQuestionText(isGroup, senderName, channelName, originChannelType);
|
||||
const options = normalizeOptions(buildApprovalOptions(agentGroups, delivery.userId));
|
||||
|
||||
// agent_group_id is NOT NULL bookkeeping (the schema predates the global
|
||||
// approver chain); it no longer drives approver resolution or click-auth.
|
||||
createPendingChannelApproval({
|
||||
messaging_group_id: messagingGroupId,
|
||||
agent_group_id: referenceGroup.id,
|
||||
agent_group_id: agentGroups[0].id,
|
||||
original_message: JSON.stringify(event),
|
||||
approver_user_id: delivery.userId,
|
||||
created_at: new Date().toISOString(),
|
||||
@@ -271,6 +275,38 @@ export function buildAgentSelectionOptions(
|
||||
return normalizeOptions(options);
|
||||
}
|
||||
|
||||
/**
|
||||
* The channel-registration hold as a hold-record view (the shape
|
||||
* pending_approvals rows have), so its terminal resolutions can announce
|
||||
* through the shared approval-resolved observer. The flow itself keeps its
|
||||
* own table and multi-step conversation.
|
||||
*/
|
||||
export function channelHoldView(
|
||||
row: PendingChannelApproval,
|
||||
payloadExtra: Record<string, unknown> = {},
|
||||
): PendingApproval {
|
||||
return {
|
||||
approval_id: row.messaging_group_id,
|
||||
session_id: null,
|
||||
request_id: row.messaging_group_id,
|
||||
action: 'channel_registration',
|
||||
payload: JSON.stringify({ messagingGroupId: row.messaging_group_id, ...payloadExtra }),
|
||||
created_at: row.created_at,
|
||||
agent_group_id: null,
|
||||
channel_type: null,
|
||||
platform_id: null,
|
||||
platform_message_id: null,
|
||||
expires_at: null,
|
||||
status: 'pending',
|
||||
title: row.title,
|
||||
options_json: row.options_json,
|
||||
approver_user_id: row.approver_user_id,
|
||||
eligibility: 'admins-of-scope',
|
||||
approver_scope: 'group',
|
||||
dedup_key: null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new agent group and initialize its filesystem. Handles
|
||||
* folder-name collisions with numeric suffixes.
|
||||
|
||||
@@ -1,60 +0,0 @@
|
||||
/**
|
||||
* CRUD for pending_sender_approvals — the in-flight state for the
|
||||
* request_approval unknown-sender flow. Rows are created when an unknown
|
||||
* sender writes into a wired messaging group with that policy, and are
|
||||
* deleted on admin approve (after adding the user as a member) or deny.
|
||||
*
|
||||
* UNIQUE(messaging_group_id, sender_identity) enforces in-flight dedup:
|
||||
* a retry / second message from the same unknown sender while a card is
|
||||
* still pending is silently dropped instead of spamming the admin.
|
||||
*/
|
||||
import { getDb } from '../../../db/connection.js';
|
||||
|
||||
export interface PendingSenderApproval {
|
||||
id: string;
|
||||
messaging_group_id: string;
|
||||
agent_group_id: string;
|
||||
sender_identity: string;
|
||||
sender_name: string | null;
|
||||
original_message: string;
|
||||
approver_user_id: string;
|
||||
created_at: string;
|
||||
/** Card title shown at creation and re-used by getAskQuestionRender on click. */
|
||||
title: string;
|
||||
/** Normalized options (JSON-encoded NormalizedOption[]) — same shape persisted on pending_approvals. */
|
||||
options_json: string;
|
||||
}
|
||||
|
||||
export function createPendingSenderApproval(row: PendingSenderApproval): void {
|
||||
getDb()
|
||||
.prepare(
|
||||
`INSERT INTO pending_sender_approvals (
|
||||
id, messaging_group_id, agent_group_id, sender_identity,
|
||||
sender_name, original_message, approver_user_id, created_at,
|
||||
title, options_json
|
||||
)
|
||||
VALUES (
|
||||
@id, @messaging_group_id, @agent_group_id, @sender_identity,
|
||||
@sender_name, @original_message, @approver_user_id, @created_at,
|
||||
@title, @options_json
|
||||
)`,
|
||||
)
|
||||
.run(row);
|
||||
}
|
||||
|
||||
export function getPendingSenderApproval(id: string): PendingSenderApproval | undefined {
|
||||
return getDb().prepare('SELECT * FROM pending_sender_approvals WHERE id = ?').get(id) as
|
||||
| PendingSenderApproval
|
||||
| undefined;
|
||||
}
|
||||
|
||||
export function hasInFlightSenderApproval(messagingGroupId: string, senderIdentity: string): boolean {
|
||||
const row = getDb()
|
||||
.prepare('SELECT 1 AS x FROM pending_sender_approvals WHERE messaging_group_id = ? AND sender_identity = ?')
|
||||
.get(messagingGroupId, senderIdentity) as { x: number } | undefined;
|
||||
return row !== undefined;
|
||||
}
|
||||
|
||||
export function deletePendingSenderApproval(id: string): void {
|
||||
getDb().prepare('DELETE FROM pending_sender_approvals WHERE id = ?').run(id);
|
||||
}
|
||||
@@ -32,9 +32,12 @@ import { registerResponseHandler, type ResponsePayload } from '../../response-re
|
||||
import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { log } from '../../log.js';
|
||||
import type { MessagingGroup, MessagingGroupAgent } from '../../types.js';
|
||||
import { mayResolve } from '../approvals/eligibility.js';
|
||||
import { notifyApprovalResolved, registerApprovalHandler } from '../approvals/primitive.js';
|
||||
import { canAccessAgentGroup } from './access.js';
|
||||
import {
|
||||
buildAgentSelectionOptions,
|
||||
channelHoldView,
|
||||
CHOOSE_EXISTING_VALUE,
|
||||
CONNECT_PREFIX,
|
||||
createNewAgentGroup,
|
||||
@@ -48,10 +51,9 @@ import {
|
||||
getPendingChannelApproval,
|
||||
updatePendingChannelApprovalCard,
|
||||
} from './db/pending-channel-approvals.js';
|
||||
import { deletePendingSenderApproval, getPendingSenderApproval } from './db/pending-sender-approvals.js';
|
||||
import { hasAdminPrivilege } from './db/user-roles.js';
|
||||
import { getUser, upsertUser } from './db/users.js';
|
||||
import { requestSenderApproval } from './sender-approval.js';
|
||||
import { requestSenderApproval, SENDER_ADMIT_ACTION } from './sender-approval.js';
|
||||
import { ensureUserDm } from './user-dm.js';
|
||||
|
||||
// ── Free-text name input state ──
|
||||
@@ -209,83 +211,37 @@ setSenderScopeGate(
|
||||
);
|
||||
|
||||
/**
|
||||
* Response handler for the unknown-sender approval card.
|
||||
* Approve continuation for the unknown-sender hold (a sessionless
|
||||
* pending_approvals row created by sender-approval.ts): add the sender to
|
||||
* agent_group_members and re-invoke routeInbound with the stored event — the
|
||||
* second routing attempt clears the gate because the user is now a member.
|
||||
*
|
||||
* Claim rule: questionId matches a row in pending_sender_approvals. If no
|
||||
* such row, return false so the next handler (approvals module, OneCLI,
|
||||
* interactive) gets a shot.
|
||||
*
|
||||
* Approve: add the sender to agent_group_members + re-invoke routeInbound
|
||||
* with the stored event. The second routing attempt clears the gate because
|
||||
* the user is now a member.
|
||||
*
|
||||
* Deny: delete the row (no "deny list" — a future message re-triggers a
|
||||
* fresh card per ACTION-ITEMS item 5 "no denial persistence").
|
||||
* Click authorization and the deny path are the approvals module's shared
|
||||
* response handler (mayResolve + finalizeReject). Deny just drops the hold —
|
||||
* no "deny list"; a future message re-triggers a fresh card.
|
||||
*/
|
||||
async function handleSenderApprovalResponse(payload: ResponsePayload): Promise<boolean> {
|
||||
const row = getPendingSenderApproval(payload.questionId);
|
||||
if (!row) return false;
|
||||
|
||||
// payload.userId is the raw platform userId (e.g. "6037840640"); namespace it
|
||||
// with the channel type so it matches users(id) format. Some platforms
|
||||
// (e.g. Teams "29:xxx") already include a colon — mirror resolveOrCreateUser
|
||||
// logic and only prefix when the raw id has no colon.
|
||||
const clickerId = payload.userId
|
||||
? payload.userId.includes(':')
|
||||
? payload.userId
|
||||
: `${payload.channelType}:${payload.userId}`
|
||||
: null;
|
||||
const isAuthorized =
|
||||
clickerId !== null && (clickerId === row.approver_user_id || hasAdminPrivilege(clickerId, row.agent_group_id));
|
||||
if (!isAuthorized) {
|
||||
log.warn('Unknown-sender approval click rejected — unauthorized clicker', {
|
||||
approvalId: row.id,
|
||||
clickerId,
|
||||
expectedApprover: row.approver_user_id,
|
||||
});
|
||||
return true; // claim the response so it's not unclaimed-logged, but do nothing
|
||||
}
|
||||
const approverId = clickerId;
|
||||
const approved = payload.value === 'approve';
|
||||
|
||||
if (approved) {
|
||||
addMember({
|
||||
user_id: row.sender_identity,
|
||||
agent_group_id: row.agent_group_id,
|
||||
added_by: approverId,
|
||||
added_at: new Date().toISOString(),
|
||||
});
|
||||
log.info('Unknown sender approved — member added', {
|
||||
approvalId: row.id,
|
||||
senderIdentity: row.sender_identity,
|
||||
agentGroupId: row.agent_group_id,
|
||||
approverId,
|
||||
});
|
||||
|
||||
// Clear the pending row BEFORE re-routing so the gate check on the
|
||||
// second attempt doesn't see the in-flight row and short-circuit.
|
||||
deletePendingSenderApproval(row.id);
|
||||
|
||||
try {
|
||||
const event = JSON.parse(row.original_message) as InboundEvent;
|
||||
await routeInbound(event);
|
||||
} catch (err) {
|
||||
log.error('Failed to replay message after sender approval', { approvalId: row.id, err });
|
||||
}
|
||||
return true;
|
||||
registerApprovalHandler(SENDER_ADMIT_ACTION, async ({ payload, userId }) => {
|
||||
const senderIdentity = typeof payload.senderIdentity === 'string' ? payload.senderIdentity : '';
|
||||
const agentGroupId = typeof payload.agentGroupId === 'string' ? payload.agentGroupId : '';
|
||||
if (!senderIdentity || !agentGroupId) {
|
||||
log.warn('sender_admit approved but the hold payload was malformed', { senderIdentity, agentGroupId });
|
||||
return;
|
||||
}
|
||||
|
||||
log.info('Unknown sender denied', {
|
||||
approvalId: row.id,
|
||||
senderIdentity: row.sender_identity,
|
||||
agentGroupId: row.agent_group_id,
|
||||
approverId,
|
||||
addMember({
|
||||
user_id: senderIdentity,
|
||||
agent_group_id: agentGroupId,
|
||||
added_by: userId,
|
||||
added_at: new Date().toISOString(),
|
||||
});
|
||||
deletePendingSenderApproval(row.id);
|
||||
return true;
|
||||
}
|
||||
log.info('Unknown sender approved — member added', { senderIdentity, agentGroupId, approverId: userId });
|
||||
|
||||
registerResponseHandler(handleSenderApprovalResponse);
|
||||
try {
|
||||
await routeInbound(payload.event as InboundEvent);
|
||||
} catch (err) {
|
||||
log.error('Failed to replay message after sender approval', { senderIdentity, agentGroupId, err });
|
||||
}
|
||||
});
|
||||
|
||||
// ── Unknown-channel registration flow ──
|
||||
|
||||
@@ -311,14 +267,21 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
|
||||
const row = getPendingChannelApproval(payload.questionId);
|
||||
if (!row) return false;
|
||||
|
||||
// payload.userId is the raw platform userId (e.g. "6037840640"); namespace it
|
||||
// with the channel type so it matches users(id) format. Some platforms
|
||||
// (e.g. Teams "29:xxx") already include a colon — mirror resolveOrCreateUser
|
||||
// logic and only prefix when the raw id has no colon.
|
||||
const clickerId = payload.userId
|
||||
? payload.userId.includes(':')
|
||||
? payload.userId
|
||||
: `${payload.channelType}:${payload.userId}`
|
||||
: null;
|
||||
const isAuthorized =
|
||||
clickerId !== null && (clickerId === row.approver_user_id || hasAdminPrivilege(clickerId, row.agent_group_id));
|
||||
if (!isAuthorized) {
|
||||
// Channel registration creates groups and wirings — approver eligibility is
|
||||
// owner / global admin (plus the delivered-to approver), never an admin
|
||||
// scoped to whichever group happens to sort first.
|
||||
if (
|
||||
!mayResolve({ kind: 'admins-of-scope', agentGroupId: null, deliveredTo: row.approver_user_id }, 'group', clickerId)
|
||||
) {
|
||||
log.warn('Channel registration click rejected — unauthorized clicker', {
|
||||
messagingGroupId: row.messaging_group_id,
|
||||
clickerId,
|
||||
@@ -326,12 +289,18 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
|
||||
});
|
||||
return true;
|
||||
}
|
||||
const approverId = clickerId;
|
||||
const approverId = clickerId as string;
|
||||
|
||||
// ── Reject / Cancel ──
|
||||
if (payload.value === REJECT_VALUE) {
|
||||
setMessagingGroupDeniedAt(row.messaging_group_id, new Date().toISOString());
|
||||
deletePendingChannelApproval(row.messaging_group_id);
|
||||
await notifyApprovalResolved({
|
||||
approval: channelHoldView(row),
|
||||
session: null,
|
||||
outcome: 'reject',
|
||||
userId: approverId,
|
||||
});
|
||||
log.info('Channel registration denied', {
|
||||
messagingGroupId: row.messaging_group_id,
|
||||
approverId,
|
||||
@@ -503,6 +472,12 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
|
||||
}
|
||||
|
||||
deletePendingChannelApproval(row.messaging_group_id);
|
||||
await notifyApprovalResolved({
|
||||
approval: channelHoldView(row, { targetAgentGroupId }),
|
||||
session: null,
|
||||
outcome: 'approve',
|
||||
userId: approverId,
|
||||
});
|
||||
|
||||
try {
|
||||
await routeInbound(event);
|
||||
@@ -603,6 +578,12 @@ registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
|
||||
}
|
||||
|
||||
deletePendingChannelApproval(row.messaging_group_id);
|
||||
await notifyApprovalResolved({
|
||||
approval: channelHoldView(row, { targetAgentGroupId: ag.id, createdAgentGroup: true }),
|
||||
session: null,
|
||||
outcome: 'approve',
|
||||
userId,
|
||||
});
|
||||
|
||||
try {
|
||||
await routeInbound(originalEvent);
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
/**
|
||||
* Integration tests for the unknown-sender request_approval flow
|
||||
* (ACTION-ITEMS item 5).
|
||||
* (ACTION-ITEMS item 5), folded onto the approvals primitive: the hold is a
|
||||
* sessionless pending_approvals row (action 'sender_admit') resolved by the
|
||||
* approvals module's shared response handler.
|
||||
*
|
||||
* Covers:
|
||||
* - request_approval policy fires `requestSenderApproval` on first unknown
|
||||
@@ -9,7 +11,9 @@
|
||||
* silently dropped (no second card, no second row)
|
||||
* - Approve path: member added, original message replayed via routeInbound,
|
||||
* container woken
|
||||
* - Deny path: pending row deleted, no member added
|
||||
* - Deny path: pending hold deleted, no member added
|
||||
* - Click authorization: named-or-admin (delivered approver or any admin of
|
||||
* the group's chain); strangers can't self-admit
|
||||
*/
|
||||
import fs from 'fs';
|
||||
import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
|
||||
@@ -28,12 +32,14 @@ vi.mock('../../container-runner.js', () => ({
|
||||
killContainer: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock delivery adapter — record card deliveries for assertions.
|
||||
// Mock delivery adapter — record card deliveries for assertions. The approvals
|
||||
// barrel also pulls onDeliveryAdapterReady from this module at import time.
|
||||
const deliverMock = vi.fn().mockResolvedValue('plat-msg-id');
|
||||
vi.mock('../../delivery.js', () => ({
|
||||
getDeliveryAdapter: () => ({
|
||||
deliver: deliverMock,
|
||||
}),
|
||||
onDeliveryAdapterReady: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock ensureUserDm to return the approver's existing messaging group
|
||||
@@ -69,10 +75,12 @@ beforeEach(async () => {
|
||||
const db = initTestDb();
|
||||
runMigrations(db);
|
||||
|
||||
// Side-effect imports: register hooks (permissions module) AFTER the
|
||||
// mocks are in place so the access gate / response handler pick up the
|
||||
// mocked delivery + user-dm helpers.
|
||||
// Side-effect imports: register hooks AFTER the mocks are in place so the
|
||||
// access gate / response handler pick up the mocked delivery + user-dm
|
||||
// helpers. The approvals barrel registers the shared response handler that
|
||||
// resolves sender_admit holds.
|
||||
await import('./index.js');
|
||||
await import('../approvals/index.js');
|
||||
|
||||
// Fixtures: agent group, messaging group with request_approval, wiring,
|
||||
// owner + DM messaging group for approver delivery.
|
||||
@@ -152,6 +160,20 @@ function stranger(text: string) {
|
||||
};
|
||||
}
|
||||
|
||||
async function pendingSenderHold(): Promise<{ approval_id: string } | undefined> {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
return getDb().prepare("SELECT approval_id FROM pending_approvals WHERE action = 'sender_admit'").get() as
|
||||
| { approval_id: string }
|
||||
| undefined;
|
||||
}
|
||||
|
||||
async function senderHoldCount(): Promise<number> {
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
return (
|
||||
getDb().prepare("SELECT COUNT(*) AS c FROM pending_approvals WHERE action = 'sender_admit'").get() as { c: number }
|
||||
).c;
|
||||
}
|
||||
|
||||
describe('unknown-sender request_approval flow', () => {
|
||||
it('delivers an approval card on first unknown message', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
@@ -168,11 +190,26 @@ describe('unknown-sender request_approval flow', () => {
|
||||
expect(kind).toBe('chat-sdk');
|
||||
const payload = JSON.parse(content as string);
|
||||
expect(payload.type).toBe('ask_question');
|
||||
expect(payload.questionId).toMatch(/^nsa-/);
|
||||
expect(payload.questionId).toMatch(/^appr-/);
|
||||
expect(payload.title).toBe('👤 New sender');
|
||||
expect(payload.options.map((o: { value: string }) => o.value)).toEqual(['approve', 'reject']);
|
||||
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const rows = getDb().prepare('SELECT * FROM pending_sender_approvals').all();
|
||||
const rows = getDb().prepare("SELECT * FROM pending_approvals WHERE action = 'sender_admit'").all() as Array<{
|
||||
session_id: string | null;
|
||||
agent_group_id: string;
|
||||
eligibility: string;
|
||||
approver_user_id: string;
|
||||
dedup_key: string;
|
||||
}>;
|
||||
expect(rows).toHaveLength(1);
|
||||
// Hold-record contract: sessionless, anchored to the agent group,
|
||||
// named-or-admin eligibility with the delivered approver recorded.
|
||||
expect(rows[0].session_id).toBeNull();
|
||||
expect(rows[0].agent_group_id).toBe('ag-1');
|
||||
expect(rows[0].eligibility).toBe('admins-of-scope');
|
||||
expect(rows[0].approver_user_id).toBe('telegram:owner');
|
||||
expect(rows[0].dedup_key).toBe('sender_admit:mg-chat:tg:stranger');
|
||||
});
|
||||
|
||||
it('dedups a second message from the same stranger while pending', async () => {
|
||||
@@ -183,9 +220,7 @@ describe('unknown-sender request_approval flow', () => {
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
expect(deliverMock).toHaveBeenCalledTimes(1);
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const count = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number }).c;
|
||||
expect(count).toBe(1);
|
||||
expect(await senderHoldCount()).toBe(1);
|
||||
});
|
||||
|
||||
it('approve → adds member and replays the original message', async () => {
|
||||
@@ -197,17 +232,16 @@ describe('unknown-sender request_approval flow', () => {
|
||||
await routeInbound(stranger('please let me in'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
|
||||
const pending = await pendingSenderHold();
|
||||
expect(pending).toBeDefined();
|
||||
|
||||
// Fire the approve click through the response-handler chain.
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.id,
|
||||
questionId: pending!.approval_id,
|
||||
value: 'approve',
|
||||
// Chat SDK's onAction surfaces the raw platform userId (e.g. Telegram
|
||||
// chat id). The permissions handler namespaces it with channelType to
|
||||
// chat id). The response handler namespaces it with channelType to
|
||||
// match users(id).
|
||||
userId: 'owner',
|
||||
channelType: 'telegram',
|
||||
@@ -218,33 +252,32 @@ describe('unknown-sender request_approval flow', () => {
|
||||
}
|
||||
|
||||
// Member row added for the stranger against the wired agent group.
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const member = getDb()
|
||||
.prepare('SELECT 1 AS x FROM agent_group_members WHERE user_id = ? AND agent_group_id = ?')
|
||||
.get('tg:stranger', 'ag-1');
|
||||
expect(member).toBeDefined();
|
||||
|
||||
// Pending row cleared.
|
||||
const stillPending = getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number };
|
||||
expect(stillPending.c).toBe(0);
|
||||
// Pending hold cleared.
|
||||
expect(await senderHoldCount()).toBe(0);
|
||||
|
||||
// Message replayed + container woken.
|
||||
expect(wakeContainer).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('deny → deletes the pending row without adding a member', async () => {
|
||||
it('deny → deletes the pending hold without adding a member', async () => {
|
||||
const { routeInbound } = await import('../../router.js');
|
||||
const { getResponseHandlers } = await import('../../response-registry.js');
|
||||
|
||||
await routeInbound(stranger('hello'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
|
||||
const pending = await pendingSenderHold();
|
||||
expect(pending).toBeDefined();
|
||||
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.id,
|
||||
questionId: pending!.approval_id,
|
||||
value: 'reject',
|
||||
userId: 'owner', // raw platform id — handler namespaces with channelType
|
||||
channelType: 'telegram',
|
||||
@@ -254,8 +287,8 @@ describe('unknown-sender request_approval flow', () => {
|
||||
if (claimed) break;
|
||||
}
|
||||
|
||||
const count = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number }).c;
|
||||
expect(count).toBe(0);
|
||||
expect(await senderHoldCount()).toBe(0);
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const member = getDb()
|
||||
.prepare('SELECT 1 AS x FROM agent_group_members WHERE user_id = ? AND agent_group_id = ?')
|
||||
.get('tg:stranger', 'ag-1');
|
||||
@@ -270,8 +303,7 @@ describe('unknown-sender request_approval flow', () => {
|
||||
await routeInbound(stranger('can I play'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
|
||||
const pending = await pendingSenderHold();
|
||||
expect(pending).toBeDefined();
|
||||
|
||||
// A random user (not the stranger, not the owner, not an admin) tries to
|
||||
@@ -279,7 +311,7 @@ describe('unknown-sender request_approval flow', () => {
|
||||
// rejected without admitting them.
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.id,
|
||||
questionId: pending!.approval_id,
|
||||
value: 'approve',
|
||||
userId: 'random-bystander', // not owner, not admin
|
||||
channelType: 'telegram',
|
||||
@@ -290,18 +322,17 @@ describe('unknown-sender request_approval flow', () => {
|
||||
}
|
||||
|
||||
// No member added for the stranger.
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const member = getDb()
|
||||
.prepare('SELECT 1 AS x FROM agent_group_members WHERE user_id = ? AND agent_group_id = ?')
|
||||
.get('tg:stranger', 'ag-1');
|
||||
expect(member).toBeUndefined();
|
||||
|
||||
// Pending row is still there — a legitimate approver can still act on it.
|
||||
const stillPending = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number })
|
||||
.c;
|
||||
expect(stillPending).toBe(1);
|
||||
// Pending hold is still there — a legitimate approver can still act on it.
|
||||
expect(await senderHoldCount()).toBe(1);
|
||||
});
|
||||
|
||||
it('accepts a click from a global admin even if they are not the designated approver', async () => {
|
||||
it('accepts a click from a global admin even if they are not the delivered approver', async () => {
|
||||
// Pre-seed a separate admin user so we can click as them.
|
||||
upsertUser({ id: 'telegram:admin-bob', kind: 'telegram', display_name: 'Bob', created_at: now() });
|
||||
grantRole({
|
||||
@@ -318,14 +349,13 @@ describe('unknown-sender request_approval flow', () => {
|
||||
await routeInbound(stranger('knock knock'));
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
|
||||
const pending = await pendingSenderHold();
|
||||
expect(pending).toBeDefined();
|
||||
|
||||
// Admin clicks approve (not the designated approver, which was owner).
|
||||
// Admin clicks approve (not the delivered approver, which was the owner).
|
||||
for (const handler of getResponseHandlers()) {
|
||||
const claimed = await handler({
|
||||
questionId: pending.id,
|
||||
questionId: pending!.approval_id,
|
||||
value: 'approve',
|
||||
userId: 'admin-bob',
|
||||
channelType: 'telegram',
|
||||
@@ -336,6 +366,7 @@ describe('unknown-sender request_approval flow', () => {
|
||||
}
|
||||
|
||||
// Stranger admitted thanks to the admin's authority.
|
||||
const { getDb } = await import('../../db/connection.js');
|
||||
const member = getDb()
|
||||
.prepare('SELECT 1 AS x FROM agent_group_members WHERE user_id = ? AND agent_group_id = ?')
|
||||
.get('tg:stranger', 'ag-1');
|
||||
|
||||
@@ -3,44 +3,38 @@
|
||||
*
|
||||
* When `messaging_groups.unknown_sender_policy = 'request_approval'` and a
|
||||
* non-member writes into a wired chat, the access gate drops the routing
|
||||
* attempt and calls `requestSenderApproval` to:
|
||||
* attempt and calls `requestSenderApproval`, which holds through the
|
||||
* approvals primitive (action 'sender_admit'):
|
||||
*
|
||||
* 1. Pick an eligible approver (owner / admin of the agent group).
|
||||
* 2. Open / reuse a DM to that approver on a reachable channel.
|
||||
* 3. Deliver an Approve / Deny card.
|
||||
* 4. Record a pending_sender_approvals row that holds the original message
|
||||
* so it can be re-routed on approve.
|
||||
* - approver eligibility: the agent group's admin chain, plus the specific
|
||||
* admin the card was delivered to (named-or-admin);
|
||||
* - in-flight dedup via the hold's dedup key — a retry / rapid second
|
||||
* message from the same unknown sender is silently dropped (no duplicate
|
||||
* card), replacing the old sender table's UNIQUE(mg, sender);
|
||||
* - the hold is sessionless: there is no agent session to notify, so
|
||||
* failure modes (no approver, no reachable DM, no adapter) log and leave
|
||||
* no row, letting a future attempt retry.
|
||||
*
|
||||
* On approve: the handler in index.ts adds an agent_group_members row for
|
||||
* the sender and re-invokes routeInbound with the stored event — the second
|
||||
* routing attempt passes the gate because the user is now a member.
|
||||
*
|
||||
* Failure modes (logged + row NOT created, so the dedup gate lets a future
|
||||
* attempt try again):
|
||||
* - No eligible approver in user_roles — fresh install, no owner yet.
|
||||
* - Approver has no reachable DM (no user_dms row + channel can't
|
||||
* openDM) — e.g. owner hasn't registered on any channel we're wired to.
|
||||
* - Delivery adapter missing.
|
||||
*
|
||||
* Dedup: `pending_sender_approvals` has UNIQUE(messaging_group_id,
|
||||
* sender_identity). A retry / rapid second message from the same unknown
|
||||
* sender is silently dropped (no duplicate card sent).
|
||||
* On approve: the 'sender_admit' handler in index.ts adds an
|
||||
* agent_group_members row for the sender and re-invokes routeInbound with the
|
||||
* stored event — the second routing attempt passes the gate because the user
|
||||
* is now a member. On deny: the shared reject path just drops the hold (no
|
||||
* denial persistence — a future message re-triggers a fresh card).
|
||||
*/
|
||||
import { normalizeOptions, type RawOption } from '../../channels/ask-question.js';
|
||||
import type { RawOption } from '../../channels/ask-question.js';
|
||||
import { getMessagingGroup } from '../../db/messaging-groups.js';
|
||||
import { getDeliveryAdapter } from '../../delivery.js';
|
||||
import { log } from '../../log.js';
|
||||
import type { InboundEvent } from '../../channels/adapter.js';
|
||||
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
|
||||
import { createPendingSenderApproval, hasInFlightSenderApproval } from './db/pending-sender-approvals.js';
|
||||
import { requestApproval } from '../approvals/primitive.js';
|
||||
|
||||
const APPROVAL_OPTIONS: RawOption[] = [
|
||||
{ label: 'Allow', selectedLabel: '✅ Allowed', value: 'approve', style: 'primary' },
|
||||
{ label: 'Deny', selectedLabel: '❌ Denied', value: 'reject', style: 'danger' },
|
||||
];
|
||||
|
||||
function generateId(): string {
|
||||
return `nsa-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
export const SENDER_ADMIT_ACTION = 'sender_admit';
|
||||
|
||||
export function senderAdmitDedupKey(messagingGroupId: string, senderIdentity: string): string {
|
||||
return `${SENDER_ADMIT_ACTION}:${messagingGroupId}:${senderIdentity}`;
|
||||
}
|
||||
|
||||
export interface RequestSenderApprovalInput {
|
||||
@@ -54,102 +48,20 @@ export interface RequestSenderApprovalInput {
|
||||
export async function requestSenderApproval(input: RequestSenderApprovalInput): Promise<void> {
|
||||
const { messagingGroupId, agentGroupId, senderIdentity, senderName, event } = input;
|
||||
|
||||
// In-flight dedup: don't spam the admin if the same unknown sender
|
||||
// retries while a card is already pending.
|
||||
if (hasInFlightSenderApproval(messagingGroupId, senderIdentity)) {
|
||||
log.debug('Unknown-sender approval already in flight — dropping retry', {
|
||||
messagingGroupId,
|
||||
senderIdentity,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const approvers = pickApprover(agentGroupId);
|
||||
if (approvers.length === 0) {
|
||||
log.warn('Unknown-sender approval skipped — no owner or admin configured', {
|
||||
messagingGroupId,
|
||||
agentGroupId,
|
||||
senderIdentity,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const originMg = getMessagingGroup(messagingGroupId);
|
||||
const originChannelType = originMg?.channel_type ?? '';
|
||||
const target = await pickApprovalDelivery(approvers, originChannelType);
|
||||
if (!target) {
|
||||
log.warn('Unknown-sender approval skipped — no DM channel for any approver', {
|
||||
messagingGroupId,
|
||||
agentGroupId,
|
||||
senderIdentity,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const approvalId = generateId();
|
||||
const senderDisplay = senderName && senderName.length > 0 ? senderName : senderIdentity;
|
||||
const originName = originMg?.name ?? `a ${originChannelType} channel`;
|
||||
const originName = originMg?.name ?? `a ${originMg?.channel_type ?? ''} channel`;
|
||||
|
||||
const title = '👤 New sender';
|
||||
const question = `${senderDisplay} wants to talk to your agent in ${originName}. Allow?`;
|
||||
const options = normalizeOptions(APPROVAL_OPTIONS);
|
||||
|
||||
createPendingSenderApproval({
|
||||
id: approvalId,
|
||||
messaging_group_id: messagingGroupId,
|
||||
agent_group_id: agentGroupId,
|
||||
sender_identity: senderIdentity,
|
||||
sender_name: senderName,
|
||||
original_message: JSON.stringify(event),
|
||||
approver_user_id: target.userId,
|
||||
created_at: new Date().toISOString(),
|
||||
title,
|
||||
options_json: JSON.stringify(options),
|
||||
await requestApproval({
|
||||
agentGroupId,
|
||||
agentName: senderDisplay,
|
||||
action: SENDER_ADMIT_ACTION,
|
||||
payload: { messagingGroupId, agentGroupId, senderIdentity, senderName, event },
|
||||
title: '👤 New sender',
|
||||
question: `${senderDisplay} wants to talk to your agent in ${originName}. Allow?`,
|
||||
options: APPROVAL_OPTIONS,
|
||||
dedupKey: senderAdmitDedupKey(messagingGroupId, senderIdentity),
|
||||
recordDeliveredApprover: true,
|
||||
originChannelType: originMg?.channel_type ?? '',
|
||||
});
|
||||
|
||||
const adapter = getDeliveryAdapter();
|
||||
if (!adapter) {
|
||||
// Without a delivery adapter, the card can't be sent. Log + leave the
|
||||
// row in place so the admin can see it via DB or manual tooling; the
|
||||
// dedup gate will suppress further cards until it's cleared.
|
||||
log.error('Unknown-sender approval row created but no delivery adapter is wired', {
|
||||
approvalId,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await adapter.deliver(
|
||||
target.messagingGroup.channel_type,
|
||||
target.messagingGroup.platform_id,
|
||||
null,
|
||||
'chat-sdk',
|
||||
JSON.stringify({
|
||||
type: 'ask_question',
|
||||
questionId: approvalId,
|
||||
title,
|
||||
question,
|
||||
options,
|
||||
}),
|
||||
);
|
||||
log.info('Unknown-sender approval card delivered', {
|
||||
approvalId,
|
||||
senderIdentity,
|
||||
approver: target.userId,
|
||||
messagingGroupId,
|
||||
agentGroupId,
|
||||
});
|
||||
} catch (err) {
|
||||
log.error('Unknown-sender approval card delivery failed', {
|
||||
approvalId,
|
||||
err,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Option value the admin clicked that means "allow" — shared with the
|
||||
* response handler so the two sides can't drift.
|
||||
*/
|
||||
export const APPROVE_VALUE = 'approve';
|
||||
export const REJECT_VALUE = 'reject';
|
||||
|
||||
@@ -20,6 +20,10 @@ import { writeSessionMessage } from '../../session-manager.js';
|
||||
import type { ApprovalHandler } from '../approvals/index.js';
|
||||
|
||||
export const applyInstallPackages: ApprovalHandler = async ({ session, payload, userId, notify }) => {
|
||||
if (!session) {
|
||||
log.warn('install_packages approval resolved without a session — dropping');
|
||||
return;
|
||||
}
|
||||
const agentGroup = getAgentGroup(session.agent_group_id);
|
||||
if (!agentGroup) {
|
||||
notify('install_packages approved but agent group missing.');
|
||||
@@ -83,6 +87,10 @@ export const applyInstallPackages: ApprovalHandler = async ({ session, payload,
|
||||
};
|
||||
|
||||
export const applyAddMcpServer: ApprovalHandler = async ({ session, payload, userId, notify }) => {
|
||||
if (!session) {
|
||||
log.warn('add_mcp_server approval resolved without a session — dropping');
|
||||
return;
|
||||
}
|
||||
const agentGroup = getAgentGroup(session.agent_group_id);
|
||||
if (!agentGroup) {
|
||||
notify('add_mcp_server approved but agent group missing.');
|
||||
|
||||
+25
-1
@@ -189,6 +189,20 @@ export interface PendingQuestion {
|
||||
|
||||
// ── Pending approvals (central DB) ──
|
||||
|
||||
/**
|
||||
* Who may resolve a hold. `exclusive`: only the named user (an a2a policy's
|
||||
* approver). `admins-of-scope`: the admin chain of the anchoring agent group
|
||||
* (owners + global admins when the anchor is null), plus the user the card
|
||||
* was delivered to when recorded. Evaluation lives in
|
||||
* src/modules/approvals/eligibility.ts (`mayResolve`).
|
||||
*/
|
||||
export type ApproverEligibility =
|
||||
| { kind: 'exclusive'; approverUserId: string }
|
||||
| { kind: 'admins-of-scope'; agentGroupId: string | null; deliveredTo: string | null };
|
||||
|
||||
/** Blast radius of a held action: 'global' holds require an owner or global admin to resolve. */
|
||||
export type ApproverScope = 'group' | 'global';
|
||||
|
||||
export interface PendingApproval {
|
||||
approval_id: string;
|
||||
session_id: string | null;
|
||||
@@ -209,8 +223,18 @@ export interface PendingApproval {
|
||||
status: 'pending' | 'approved' | 'rejected' | 'expired' | 'awaiting_reason';
|
||||
title: string;
|
||||
options_json: string;
|
||||
/** When set, only this exact user may resolve the approval. */
|
||||
/**
|
||||
* Named approver. Under `eligibility: 'exclusive'` only this exact user may
|
||||
* resolve the approval; under 'admins-of-scope' it records the user the card
|
||||
* was delivered to, who may resolve alongside the scope's admins.
|
||||
*/
|
||||
approver_user_id: string | null;
|
||||
/** Who may resolve this hold — see modules/approvals/eligibility.ts. */
|
||||
eligibility: 'exclusive' | 'admins-of-scope';
|
||||
/** Blast radius: 'global' holds require an owner or global admin to resolve. */
|
||||
approver_scope: 'group' | 'global';
|
||||
/** In-flight dedup key: while a row carries this key, a repeat request with the same key is dropped. */
|
||||
dedup_key: string | null;
|
||||
}
|
||||
|
||||
// ── Agent destinations (central DB) ──
|
||||
|
||||
Reference in New Issue
Block a user