Compare commits

...

6 Commits

Author SHA1 Message Date
Moshe Krupper 3a16386377 refactor: drop the rule-source seam — the baseline is the decision
Policy-as-data (tighten-only rule sources, the strictest-wins lattice,
hold∧hold approver intersection) had exactly one production consumer:
the a2a agent_message_policies check, living in the same file as the
a2a.send baseline it composed with. Fold that check into the baseline
in its original order (destination denies precede the policy hold, so
the ghost-policy edge still denies) and delete the machinery it no
longer justifies: rules.ts, compose(), intersectApproverRules, the
RuleDecision type, and the lattice/intersection test blocks.

Zero behavior change — the a2a integration tests (policy hold with
exclusive approver, ghost-policy deny, self-send, grants, D3) pass
unchanged. guard() collapses to: catalog lookup → baseline → grant
check → fail closed.

The deferred machinery returns in guarded-actions phase 3, where the
generalized rules table arrives with its first operator-visible
consumer (decisions 3/4 in the hub doc stay decided; only "ships in
v1" is amended).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-08 16:37:52 +03:00
Moshe Krupper 59067c6533 refactor: rename eligibility → approver rule
Pure rename, no behavior change: the hold's who-may-resolve rule is now
named consistently with its row siblings — approver_user_id (who got the
card), approver_rule (who may act: exclusive | admins-of-scope),
approver_scope (rank floor).

- type ApproverEligibility → ApproverRule; eligibilityOf → approverRuleOf;
  guard decisions carry approverRule
- pending_approvals column eligibility → approver_rule (migration 019 is
  unmerged, renamed in place: 019-holds-approver-rule)
- src/modules/approvals/eligibility.ts → approver-rule.ts (+ test files)

Note: the hub decisions doc says "approver eligibility" (decision 4) — same
concept, this is the in-tree spelling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-08 15:57:08 +03:00
Moshe Krupper f6fd4340ed feat: boot-time guard conformance — refuse to start on unmapped privileged registrations
CI only protects code that goes through the repo's CI, but NanoClaw's
extension model is skill-installed code: /add-* skills register handlers on
machines where the test suite never runs. The conformance walk moves to a
shared core module (src/guard-conformance.ts — one exemption list, one
walk) and now runs twice: in the conformance test, and at boot before the
host accepts a message. A privileged command or delivery action registered
without a guard mapping stops the host with a clear banner (the
upgrade-tripwire posture) — a bad skill install crashes at install time,
when the installing agent is watching, instead of running unguarded.

Companion hardening: re-registering a guard-wrapped delivery action WITHOUT
a guard spec now throws at registration. Previously that silently replaced
the guarded handler while the catalog — and therefore both conformance
walks — still reported the action as guarded (green check, unguarded live
path). Re-registering with a spec stays allowed; plain-over-plain overwrite
behavior is unchanged.

Limits, unchanged: the walk verifies declared mappings, it cannot infer
which actions are privileged — that declaration stays on the author, with
EXEMPT_DELIVERY_ACTIONS as the loud, reviewable escape hatch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-08 13:16:01 +03:00
Moshe Krupper d499153df0 feat: approval-requested observer — every hold creation announces (OneCLI 4/4)
The creation-side sibling of the resolved observer. requestApproval already
covered five hold families; the OneCLI credential bridge's hold insert and
channel registration's card creation were the two moments with no shared
seam — an observer (the audit skill) would have needed one hand-made wrap in
each flow. Now every hold record announces its creation through
registerApprovalRequestedHandler / notifyApprovalRequested, carrying the
full hold row (synthesized view for channel registration), the requesting
session (null when sessionless), and the approver the card was delivered
to. With the resolution notifies from phase 1, all four OneCLI lifecycle
moments (hold insert, click, expiry, boot sweep) converge on the two shared
hooks — zero touch points inside the bridge.

Pure observer: callbacks fail isolated and never affect the hold; no
decision outcomes change. Rerouting OneCLI's hold creation through
requestApproval itself remains deliberately out (decision 5 falsified that
merge; decision 7's gateway interposition awaits its own call) — the bridge
keeps its oa- short ids, two-button card, promise lease, and expiry timer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-08 01:22:14 +03:00
Moshe Krupper 94da3542f0 feat: guard seam — decision function, registration wrapping, grant-carrying replay (guarded-actions phase 2)
Every privileged action crossing the container or channel boundary now
passes one decision function before it executes (hub:
engineering/discovery/guarded-actions-decisions +
engineering/requirements/guarded-actions, phase 2).

- src/guard/ — domain-free leaf: guard({action, actor, resource?, payload,
  grant?}) → allow | hold(eligibility, scope, reason) | deny(reason).
  Registration-derived action catalog; tighten-only rule sources composed
  with the structural baseline under the strictest-wins lattice
  (deny > hold > allow; hold∧hold intersects approver eligibility, empty
  intersections escalate to the global chain). Fails closed.
- Registration wrapping on all four registries: every ncl command derives a
  catalog entry in register() (dispatch consults the guard; auto-fill,
  the sessions-get oracle and post-handler row filtering stay as mechanics);
  registerDeliveryAction wraps create_agent + self-mod with domain guard
  specs (precheck / hold builder / deny notify — the wrapped path is the
  only path); registerResponseHandler and registerMessageInterceptor wrap
  the channel-registration click and free-text name capture (the D4
  interceptor half — the reply is re-checked against the same eligibility).
- Structural baselines moved verbatim into module-edge guard adapters:
  cli_scope enforcement (src/cli/guard.ts), create_agent's scope branch
  (a2a guard.ts — out of create-agent.ts), a2a self-send/destination/target
  checks, self-mod unconditional hold, unknown_sender_policy, host as
  trusted caller in code. agent_message_policies becomes the guard's first
  rule source; the ghost-policy edge (policy row, no destination) still
  DENIES — deny beats hold, exactly today's outcome.
- Grant-carrying replay: approved: true is deleted. Approval handlers
  receive the verified approval row (ApprovalHandlerContext.approval) and
  re-enter their entry points with it as the grant; the guard treats a live,
  matching grant as hold-satisfied but re-runs the structural baseline.
  Absorbed defect fixes (intentional decision-outcome changes):
  D2 — a malformed replay caller context refuses the replay, never falls
  back to {caller: 'host'}; D3 — the a2a approve handler re-enters the
  guarded route, so approve-then-revoke no longer delivers.
- Conformance test walks the command + delivery-action registries: every
  mutating entry must map to a guard catalog entry (scheduling self-actions
  and the cli_request bridge are the declared exemption class).

Zero rules ship; the seeded posture is today's behavior. No other decision
outcomes change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 21:29:29 +03:00
Moshe Krupper de7e596aa5 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>
2026-07-07 21:00:40 +03:00
53 changed files with 2852 additions and 756 deletions
+4
View File
@@ -4,6 +4,10 @@ All notable changes to NanoClaw will be documented in this file.
## [Unreleased]
- **Guard conformance is now a boot invariant, not just a CI test.** The host refuses to start (upgrade-tripwire posture: clear banner + exit 1) if any privileged command or delivery action is registered without a guard-catalog mapping. CI can't see skill-installed code — `/add-*` skills register handlers on machines where the test suite never runs — so the same registry walk the conformance test uses (`src/guard-conformance.ts`, one shared exemption list) now runs in `main()` before the host accepts a message, and a bad registration crashes at skill-install time instead of running unguarded. Companion hardening: re-registering a guard-wrapped delivery action *without* a guard spec now throws at registration — previously that silently replaced the guarded handler while the catalog still reported the action as guarded.
- **Approval-requested observer — hold creation announces like resolution does.** `registerApprovalRequestedHandler` is the creation-side sibling of `registerApprovalResolvedHandler`: it fires once whenever a hold record comes into existence, whichever stack created it — `requestApproval` (cli_command, create_agent, self-mod, a2a, sender admission), the OneCLI credential bridge (its own rows/ids/card — the one lifecycle moment that previously had no shared seam), and channel registration (as a synthesized hold view). Pure observer: callbacks fail isolated, no decision outcome changes. Observers now see the full hold lifecycle (requested → approve/reject/expire/sweep) with zero touch points inside the flows.
- **The guard seam (guarded-actions phase 2).** Every privileged action crossing the container or channel boundary now passes ONE decision function — `guard()` in the new `src/guard/` leaf — before it executes: `allow`, `hold` (through the approvals primitive), or `deny`. The four handler registries (ncl commands, delivery actions, response handlers, message interceptors) wrap their handlers at registration, so the guarded path is the only path by construction, and a registry-walking conformance test fails CI on any unmapped mutating entry. The structural baseline is today's checks verbatim (cli_scope enforcement, create_agent's scope branch, a2a destination ACL + `agent_message_policies` hold, self-mod unconditional hold, unknown_sender_policy, host-as-trusted-caller in code). Policy-as-data (tighten-only rule sources composing with the baseline) is deferred to phase 3, where the generalized rules table arrives with its first operator-visible consumer. Approved replays now carry the verified approval row as a **grant** — the `approved: true` boolean is deleted — and the guard re-runs the structural baseline on every replay. Two absorbed security fixes intentionally change decision outcomes: **(D2)** a malformed replay caller context refuses the replay instead of falling back to the most privileged caller; **(D3)** approving a held a2a message after its destination was revoked no longer delivers it. Zero rules ship by default — the seeded posture is today's behavior.
- **One approval contract across every hold (guarded-actions phase 1).** All approval holds now share one hold-record shape on `pending_approvals` (approver rule, approver blast-radius scope, dedup key) and ONE click-authorization rule (`mayResolve` in `src/modules/approvals/approver-rule.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 approver rule; 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.
+3 -1
View File
@@ -66,8 +66,10 @@ For ad-hoc queries from skills or scripts, use the in-tree wrapper rather than t
| `src/session-manager.ts` | Resolves sessions; opens `inbound.db` / `outbound.db`; manages heartbeat path |
| `src/container-runner.ts` | Spawns per-agent-group Docker containers with session DB + outbox mounts, OneCLI `ensureAgent` |
| `src/container-runtime.ts` | Runtime selection (Docker vs Apple containers), orphan cleanup |
| `src/guard/` | Privileged-action decision seam: `guard()` → allow \| hold \| deny. Domain-free leaf (decision function + registration-derived action catalog); domain baselines register from module-edge `guard.ts` adapters (cli, agent-to-agent, self-mod, permissions). All four handler registries wrap at registration; approved replays carry the approval row as a grant and re-check the structural baseline. Policy-as-data (tighten-only rule sources) is deferred to guarded-actions phase 3. Conformance test: `src/guard/conformance.test.ts` |
| `src/modules/permissions/access.ts` | `canAccessAgentGroup` — owner / global admin / scoped admin / member resolution against `user_roles` + `agent_group_members` |
| `src/modules/approvals/primitive.ts` | `pickApprover`, `pickApprovalDelivery`, `requestApproval`, approval-handler registry |
| `src/modules/approvals/primitive.ts` | `pickApprover`, `pickApprovalDelivery`, `requestApproval`, approval-handler registry, hold-lifecycle observers (`registerApprovalRequestedHandler` / `registerApprovalResolvedHandler` — every stack announces creation + resolution through these) |
| `src/modules/approvals/approver-rule.ts` | `mayResolve` — the one click-authorization rule (approver rule `exclusive` \| `admins-of-scope` + blast-radius scope) for every hold |
| `src/command-gate.ts` | Router-side admin command gate — queries `user_roles` directly (no env var, no container-side check) |
| `src/modules/approvals/onecli-approvals.ts` | OneCLI credentialed-action approval bridge |
| `src/modules/permissions/user-dm.ts` | Cold-DM resolution + `user_dms` cache |
+6
View File
@@ -232,6 +232,7 @@ export function registerResource(def: ResourceDef): void {
if (def.operations.list) {
register({
name: `${def.plural}-list`,
action: `${def.plural}.list`,
description: `List all ${def.plural}.`,
access: def.operations.list,
resource: def.plural,
@@ -244,6 +245,7 @@ export function registerResource(def: ResourceDef): void {
if (def.operations.get) {
register({
name: `${def.plural}-get`,
action: `${def.plural}.get`,
description: `Get a ${def.name} by ID.`,
access: def.operations.get,
resource: def.plural,
@@ -256,6 +258,7 @@ export function registerResource(def: ResourceDef): void {
if (def.operations.create) {
register({
name: `${def.plural}-create`,
action: `${def.plural}.create`,
description: `Create a new ${def.name}.`,
access: def.operations.create,
resource: def.plural,
@@ -267,6 +270,7 @@ export function registerResource(def: ResourceDef): void {
if (def.operations.update) {
register({
name: `${def.plural}-update`,
action: `${def.plural}.update`,
description: `Update a ${def.name}.`,
access: def.operations.update,
resource: def.plural,
@@ -278,6 +282,7 @@ export function registerResource(def: ResourceDef): void {
if (def.operations.delete) {
register({
name: `${def.plural}-delete`,
action: `${def.plural}.delete`,
description: `Delete a ${def.name}.`,
access: def.operations.delete,
resource: def.plural,
@@ -291,6 +296,7 @@ export function registerResource(def: ResourceDef): void {
for (const [verb, op] of Object.entries(def.customOperations)) {
register({
name: `${def.plural}-${verb.replace(/ /g, '-')}`,
action: `${def.plural}.${verb.replace(/ /g, '.')}`,
description: op.description,
access: op.access,
resource: def.plural,
+134
View File
@@ -9,6 +9,7 @@ const approvalState = vi.hoisted(() => ({
| ((args: {
session: unknown;
payload: Record<string, unknown>;
approval: Record<string, unknown>;
userId: string;
notify: (text: string) => void;
}) => Promise<void>),
@@ -18,6 +19,7 @@ const approvalState = vi.hoisted(() => ({
handler: (args: {
session: unknown;
payload: Record<string, unknown>;
approval: Record<string, unknown>;
userId: string;
notify: (text: string) => void;
}) => Promise<void>,
@@ -43,8 +45,10 @@ vi.mock('../db/agent-groups.js', () => ({
}));
const mockGetSession = vi.fn();
const mockGetPendingApproval = vi.fn();
vi.mock('../db/sessions.js', () => ({
getSession: (...args: unknown[]) => mockGetSession(...args),
getPendingApproval: (...args: unknown[]) => mockGetPendingApproval(...args),
}));
// dispatch's post-handler looks up the resource's `scopeField` via getResource.
@@ -136,6 +140,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',
@@ -461,10 +492,20 @@ describe('CLI scope enforcement', () => {
callerContext: ctx,
});
// The continuation carries the verified approval row as its grant; the
// guard accepts it only while the row is live.
const grantRow = {
approval_id: 'appr-replay-1',
action: 'cli_command',
payload: JSON.stringify(approval.payload),
};
mockGetPendingApproval.mockReturnValue(grantRow);
expect(approvalState.approvalHandler).toBeTypeOf('function');
await approvalState.approvalHandler!({
session: { id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' },
payload: approval.payload,
approval: grantRow,
userId: 'telegram:admin',
notify: vi.fn(),
});
@@ -473,6 +514,99 @@ describe('CLI scope enforcement', () => {
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1);
});
it('D2: a malformed replay caller context refuses the replay (never falls back to host)', 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 ctx = agentCtx();
await dispatch({ id: '1', command: 'approval-context-command', args: {} }, ctx);
const approval = approvalState.requestApproval.mock.calls[0][0] as { payload: Record<string, unknown> };
const grantRow = { approval_id: 'appr-replay-2', action: 'cli_command', payload: JSON.stringify(approval.payload) };
mockGetPendingApproval.mockReturnValue(grantRow);
const notify = vi.fn();
await approvalState.approvalHandler!({
session: { id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' },
payload: { ...approval.payload, callerContext: { caller: 'agent', sessionId: 42 } }, // malformed
approval: grantRow,
userId: 'telegram:admin',
notify,
});
// The command never executed — as host or anyone else — and the agent was told.
expect(approvalState.observedContexts).toEqual([]);
expect(notify).toHaveBeenCalledWith(expect.stringContaining('refused'));
});
it('a dead grant refuses the replay instead of re-executing or re-holding', 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 ctx = agentCtx();
await dispatch({ id: '1', command: 'approval-context-command', args: {} }, ctx);
const approval = approvalState.requestApproval.mock.calls[0][0] as { payload: Record<string, unknown> };
const grantRow = { approval_id: 'appr-replay-3', action: 'cli_command', payload: JSON.stringify(approval.payload) };
mockGetPendingApproval.mockReturnValue(undefined); // row already resolved/deleted
const notify = vi.fn();
await approvalState.approvalHandler!({
session: { id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' },
payload: approval.payload,
approval: grantRow,
userId: 'telegram:admin',
notify,
});
expect(approvalState.observedContexts).toEqual([]);
expect(approvalState.requestApproval).toHaveBeenCalledTimes(1); // no second card
expect(notify).toHaveBeenCalledWith(expect.stringContaining('failed'));
});
// --- 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 () => {
+51 -41
View File
@@ -3,22 +3,37 @@
* the per-session DB poller (container caller) call dispatch() with the
* same frame and a transport-supplied CallerContext.
*
* Approval gating for risky calls from the container is the only branch
* that differs by caller. Host callers and `open` commands run inline.
* Every command passes the guard before its handler runs — the decision
* (allow / hold / deny) comes from the command's catalog entry, derived at
* registration (see cli/guard.ts). Dispatch keeps the mechanics: arg
* auto-fill, the sessions-get existence oracle, parseArgs, and post-handler
* row filtering. An approved replay re-enters here carrying the verified
* approval row as its grant — the guard re-checks the structural baseline
* live, and the `approved: true` boolean no longer exists.
*/
import { getContainerConfig } from '../db/container-configs.js';
import { getAgentGroup } from '../db/agent-groups.js';
import { getSession } from '../db/sessions.js';
import { guard, type GuardActor } from '../guard/index.js';
import { log } from '../log.js';
import { registerApprovalHandler, requestApproval } from '../modules/approvals/index.js';
import type { PendingApproval } from '../types.js';
import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js';
import { getResource } from './crud.js';
import { commandGuardAction } from './guard.js';
import { lookup } from './registry.js';
type DispatchOptions = {
/** True when a command is being replayed after approval. */
approved?: boolean;
/** Verified approval row when a command is replayed after approval. */
grant?: PendingApproval;
};
function actorFor(ctx: CallerContext): GuardActor {
return ctx.caller === 'host'
? { kind: 'host' }
: { kind: 'agent', agentGroupId: ctx.agentGroupId, sessionId: ctx.sessionId };
}
export async function dispatch(
req: RequestFrame,
ctx: CallerContext,
@@ -52,44 +67,13 @@ export async function dispatch(
return err(req.id, 'unknown-command', `no command "${req.command}"`);
}
// CLI scope enforcement for agent callers
// Group-scope mechanics for agent callers (visibility, not policy — the
// decisions live in the guard baseline, cli/guard.ts).
if (ctx.caller === 'agent') {
const configRow = getContainerConfig(ctx.agentGroupId);
const cliScope = configRow?.cli_scope ?? 'group';
if (cliScope === 'disabled') {
return err(req.id, 'forbidden', 'CLI access is disabled for this agent group.');
}
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)) {
return err(req.id, 'forbidden', `CLI access is scoped to this agent group. Cannot access "${cmd.resource}".`);
}
// Enforce group scope on all agent-group-related args.
// Different resources use different arg names for the agent group ID.
// Only check --id for resources where it IS the agent group ID.
const groupArgs = ['agent_group_id', 'group'] as const;
for (const key of groupArgs) {
if (req.args[key] && req.args[key] !== ctx.agentGroupId) {
return err(req.id, 'forbidden', 'CLI access is scoped to this agent group.');
}
}
if (
(cmd.resource === 'groups' || cmd.resource === 'destinations') &&
req.args.id &&
req.args.id !== ctx.agentGroupId
) {
return err(req.id, 'forbidden', 'CLI access is scoped to this agent group.');
}
// Block cli_scope changes from group-scoped agents (privilege escalation)
if (req.args.cli_scope !== undefined || req.args['cli-scope'] !== undefined) {
return err(req.id, 'forbidden', 'Cannot change cli_scope from a group-scoped agent.');
}
// Auto-fill agent-group-related args so the agent doesn't need
// to pass its own group ID explicitly.
const fill: Record<string, unknown> = {
@@ -115,7 +99,23 @@ export async function dispatch(
}
}
if (ctx.caller !== 'host' && cmd.access === 'approval' && !opts.approved) {
const decision = guard({
action: commandGuardAction(cmd),
actor: actorFor(ctx),
payload: req.args,
grant: opts.grant ?? null,
});
if (decision.effect === 'deny') {
return err(req.id, 'forbidden', decision.reason);
}
if (decision.effect === 'hold') {
if (ctx.caller !== 'agent') {
// Holds only arise for agent callers; anything else is a guard bug —
// fail closed rather than card a ghost.
return err(req.id, 'forbidden', decision.reason);
}
const session = getSession(ctx.sessionId);
if (!session) {
return err(req.id, 'handler-error', 'Session not found.');
@@ -134,6 +134,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: decision.approverScope,
});
return err(req.id, 'approval-pending', 'Approval request sent to admin. You will be notified of the result.');
@@ -192,10 +193,19 @@ export async function dispatch(
}
}
registerApprovalHandler('cli_command', async ({ payload, notify }) => {
registerApprovalHandler('cli_command', async ({ payload, approval, notify }) => {
const frame = payload.frame as RequestFrame;
const callerContext = parseCallerContext(payload.callerContext) ?? { caller: 'host' };
const response = await dispatch(frame, callerContext, { approved: true });
const callerContext = parseCallerContext(payload.callerContext);
if (!callerContext) {
// D2: a malformed caller context must refuse the replay — never fall
// back to the most privileged caller.
log.warn('cli_command replay refused — malformed caller context', { command: frame?.command });
notify(
`Your \`ncl ${frame?.command ?? '?'}\` request was approved, but its caller context could not be verified — the replay was refused.`,
);
return;
}
const response = await dispatch(frame, callerContext, { grant: approval });
if (response.ok) {
const data = typeof response.data === 'string' ? response.data : JSON.stringify(response.data, null, 2);
+116
View File
@@ -0,0 +1,116 @@
/**
* CLI guard adapter — the command registry's catalog derivation and
* structural baseline, moved verbatim out of dispatch.ts (guarded-actions
* phase 2). Declaration is registration: registry.register() derives one
* catalog entry per command from the CommandDef itself; no second file is
* edited when a command is added.
*
* The baseline carries today's decisions exactly:
* host caller → allow (the 0600 socket is the auth story — in code,
* unremovable by data);
* cli_scope 'disabled' → deny; 'group' → resource allowlist, cross-group
* arg denial, cli_scope-change denial;
* access 'approval' for agent callers → hold for the group's admin chain,
* with the action's blast radius as the approver scope (D1).
*
* Arg auto-fill, the sessions-get existence oracle, and post-handler row
* filtering stay in dispatch.ts — mechanics, not policy.
*/
import { getContainerConfig } from '../db/container-configs.js';
import { ALLOW, DENY, HOLD, type GuardedActionSpec, type GuardInput } from '../guard/index.js';
import type { ApproverScope } from '../types.js';
import type { CommandDef } from './registry.js';
/**
* 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.
*/
export const GROUP_SCOPED_RESOURCES = new Set(['groups', 'sessions', 'destinations', 'members']);
/**
* Blast radius of a held command, for the approver rule (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.
*/
export function approverScopeFor(
cmd: Pick<CommandDef, 'resource'>,
args: Record<string, unknown>,
callerAgentGroupId: string,
): ApproverScope {
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';
}
/** Dotted catalog action name for a command. */
export function commandGuardAction(cmd: Pick<CommandDef, 'name' | 'action'>): string {
return cmd.action ?? `cli.${cmd.name}`;
}
/** Catalog entry derived from a CommandDef at registration time. */
export function commandGuardSpec(cmd: CommandDef): GuardedActionSpec {
return {
action: commandGuardAction(cmd),
approvalAction: cmd.access === 'approval' ? 'cli_command' : undefined,
// Bind a cli_command grant to the exact command it was approved for.
grantMatches: (grant) => {
try {
const payload = JSON.parse(grant.payload) as { frame?: { command?: string } };
return payload.frame?.command === cmd.name;
} catch {
return false;
}
},
baseline: (input) => commandBaseline(cmd, input),
};
}
function commandBaseline(cmd: CommandDef, input: GuardInput) {
const { actor } = input;
if (actor.kind === 'host') return ALLOW('host caller (trusted socket)');
if (actor.kind !== 'agent') return DENY('CLI commands accept host or agent callers only.');
const args = input.payload;
const cliScope = getContainerConfig(actor.agentGroupId)?.cli_scope ?? 'group';
if (cliScope === 'disabled') {
return DENY('CLI access is disabled for this agent group.');
}
if (cliScope === 'group') {
// Only allow whitelisted resources and general commands (no resource, like help)
if (cmd.resource && !GROUP_SCOPED_RESOURCES.has(cmd.resource)) {
return DENY(`CLI access is scoped to this agent group. Cannot access "${cmd.resource}".`);
}
// Enforce group scope on all agent-group-related args.
// Different resources use different arg names for the agent group ID.
// Only check --id for resources where it IS the agent group ID.
for (const key of ['agent_group_id', 'group'] as const) {
if (args[key] && args[key] !== actor.agentGroupId) {
return DENY('CLI access is scoped to this agent group.');
}
}
if ((cmd.resource === 'groups' || cmd.resource === 'destinations') && args.id && args.id !== actor.agentGroupId) {
return DENY('CLI access is scoped to this agent group.');
}
// Block cli_scope changes from group-scoped agents (privilege escalation)
if (args.cli_scope !== undefined || args['cli-scope'] !== undefined) {
return DENY('Cannot change cli_scope from a group-scoped agent.');
}
}
if (cmd.access === 'approval') {
return HOLD(
{ kind: 'admins-of-scope', agentGroupId: actor.agentGroupId, deliveredTo: null },
approverScopeFor(cmd, args, actor.agentGroupId),
`agent-initiated "${cmd.name}" requires admin approval`,
);
}
return ALLOW('open command');
}
+13
View File
@@ -8,6 +8,8 @@
* registers the help commands, so the registry is populated before the host's
* CLI server accepts connections.
*/
import { registerGuardedAction } from '../guard/index.js';
import { commandGuardSpec } from './guard.js';
import type { CallerContext } from './frame.js';
export type Access = 'open' | 'approval';
@@ -16,6 +18,13 @@ export type CommandDef<TArgs = unknown, TData = unknown> = {
name: string;
description: string;
access: Access;
/**
* Dotted guard-catalog action name (e.g. `roles.grant`,
* `groups.config.add-mcp-server`). Set by registerResource from the
* resource + verb; commands registered directly (help) fall back to
* `cli.<name>`.
*/
action?: string;
/**
* The group-scope whitelist key. Under `cli_scope: 'group'` the dispatcher
* only lets an agent run commands whose `resource` is on the whitelist
@@ -43,6 +52,10 @@ export function register<TArgs, TData>(def: CommandDef<TArgs, TData>): void {
throw new Error(`CLI command "${def.name}" already registered`);
}
registry.set(def.name, def as CommandDef);
// Declaration is registration: every command gets a guard-catalog entry
// derived from its own definition — dispatch consults the guard, and the
// conformance test walks the registry against the catalog.
registerGuardedAction(commandGuardSpec(def as CommandDef));
}
export function lookup(name: string): CommandDef | undefined {
+18
View File
@@ -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: 'approver_rule',
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' },
});
+7 -6
View File
@@ -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);
-4
View File
@@ -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-approver-rule): in-flight
* pending_approvals rows created by the pre-contract code must come out with
* the approver rule 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-approver-rule migration.
runMigrations(
db,
migrations.filter((m) => m.name !== 'holds-approver-rule'),
);
});
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-approver-rule', () => {
it('backfills approver_rule 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-approver-rule
const rows = db
.prepare('SELECT approval_id, approver_rule, approver_scope, agent_group_id FROM pending_approvals')
.all() as Array<{ approval_id: string; approver_rule: string; approver_scope: string; agent_group_id: string }>;
const byId = Object.fromEntries(rows.map((r) => [r.approval_id, r]));
expect(byId['appr-a2a']).toMatchObject({
approver_rule: 'exclusive',
approver_scope: 'group',
agent_group_id: 'ag-1',
});
expect(byId['appr-cli']).toMatchObject({
approver_rule: 'admins-of-scope',
approver_scope: 'group',
agent_group_id: 'ag-1',
});
expect(byId['oa-1']).toMatchObject({ approver_rule: '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):
*
* - `approver_rule` — 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-approver-rule',
up(db) {
db.exec(`ALTER TABLE pending_approvals ADD COLUMN approver_rule 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 approver_rule = '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;`);
},
};
+2
View File
@@ -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-approver-rule.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
-15
View File
@@ -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
View File
@@ -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, approver_rule, 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, @approver_rule, @approver_scope, @dedup_key)`,
)
.run({
session_id: null,
@@ -170,11 +170,21 @@ export function createPendingApproval(
expires_at: null,
status: 'pending',
approver_user_id: null,
approver_rule: '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;
}
+20
View File
@@ -35,4 +35,24 @@ describe('delivery action registry', () => {
registerDeliveryAction('test_overwrite_action', second);
expect(getDeliveryAction('test_overwrite_action')).toBe(second);
});
it('refuses to replace a guard-wrapped action with an unguarded handler', () => {
registerDeliveryAction('test_guarded_overwrite', async () => {}, {
guardAction: 'test.guarded-overwrite',
requestHold: async () => {},
});
// Disarming the guard by re-registering without a spec must throw —
// otherwise the catalog (and the conformance walk) would still report
// the action guarded while the live path runs unguarded.
expect(() => registerDeliveryAction('test_guarded_overwrite', async () => {})).toThrow(/disarm the guard/);
// Re-registering WITH a spec stays allowed (a legitimate replacement
// keeps the action guarded).
registerDeliveryAction('test_guarded_overwrite', async () => {}, {
guardAction: 'test.guarded-overwrite',
requestHold: async () => {},
});
expect(getDeliveryAction('test_guarded_overwrite')).toBeDefined();
});
});
+113 -11
View File
@@ -20,12 +20,13 @@ import {
markDeliveryFailed,
migrateDeliveredTable,
} from './db/session-db.js';
import { guard } from './guard/index.js';
import { log } from './log.js';
import { normalizeOptions } from './channels/ask-question.js';
import { clearOutbox, openInboundDb, openOutboundDb, readOutboxFiles } from './session-manager.js';
import { pauseTypingRefreshAfterDelivery, setTypingAdapter } from './modules/typing/index.js';
import type { OutboundFile } from './channels/adapter.js';
import type { Session } from './types.js';
import type { PendingApproval, Session } from './types.js';
const ACTIVE_POLL_MS = 1000;
const SWEEP_POLL_MS = 60_000;
@@ -393,14 +394,18 @@ async function deliverMessage(
* Delivery action registry.
*
* Modules register handlers for system-kind outbound message actions via
* `registerDeliveryAction`. Core checks the registry first in
* `handleSystemAction` and falls through to the inline switch when no
* handler is registered. The switch will shrink as modules are extracted
* (scheduling, approvals, agent-to-agent) and eventually only its default
* branch remains.
* `registerDeliveryAction`. Unknown actions log "Unknown system action".
*
* Default when no handler registered and the switch doesn't match: log
* "Unknown system action" and return.
* Privileged delivery actions (create_agent, install_packages,
* add_mcp_server) register with a guard spec: the registry wraps the handler
* so the guard's decision — allow / hold / deny — stands between the
* container's outbound row and the handler body, and the wrapped path is the
* only path (the raw handler is never stored). On approve, the continuation
* re-enters the same wrapped entry carrying the approval row as its grant
* (`reenterGuardedDeliveryAction`), so the structural baseline is re-checked
* live. Plain actions (scheduling self-actions, the cli_request bridge — its
* inner commands are guarded at dispatch) register without a spec and are
* not catalog actions.
*/
export type DeliveryActionHandler = (
content: Record<string, unknown>,
@@ -408,13 +413,103 @@ export type DeliveryActionHandler = (
inDb: Database.Database,
) => Promise<void>;
const actionHandlers = new Map<string, DeliveryActionHandler>();
/** Handler shape for guard-wrapped actions — must not touch inDb (replays run without one). */
export type GuardedDeliveryHandler = (content: Record<string, unknown>, session: Session) => Promise<void>;
export function registerDeliveryAction(action: string, handler: DeliveryActionHandler): void {
export interface DeliveryGuardSpec {
/** Dotted guard-catalog action consulted before the handler runs. */
guardAction: string;
/**
* Domain validation that runs before the guard — malformed requests are
* answered (notify) without ever creating a hold. Return false to stop.
*/
precheck?: (content: Record<string, unknown>, session: Session) => boolean | Promise<boolean>;
/** Create the hold (the domain's requestApproval call — card text lives with the domain). */
requestHold: (content: Record<string, unknown>, session: Session) => Promise<void>;
/** Tell the requester about a deny. */
onDeny?: (content: Record<string, unknown>, session: Session, reason: string) => void;
}
const actionHandlers = new Map<string, DeliveryActionHandler>();
const guardedActions = new Map<string, { spec: DeliveryGuardSpec; handler: GuardedDeliveryHandler }>();
export function registerDeliveryAction(action: string, handler: DeliveryActionHandler): void;
export function registerDeliveryAction(action: string, handler: GuardedDeliveryHandler, spec: DeliveryGuardSpec): void;
export function registerDeliveryAction(
action: string,
handler: DeliveryActionHandler | GuardedDeliveryHandler,
spec?: DeliveryGuardSpec,
): void {
if (actionHandlers.has(action)) {
// Replacing a guard-wrapped action with an unguarded handler would
// disarm the guard while the catalog (and the conformance walk) still
// report it guarded — refuse. A skill that wants to extend a guarded
// action must compose at the module's exported functions instead, or
// re-register with a guard spec of its own.
if (!spec && guardedActions.has(action)) {
throw new Error(
`delivery action "${action}" is guard-wrapped; re-registering it without a guard spec would disarm the guard`,
);
}
log.warn('Delivery action handler overwritten', { action });
}
actionHandlers.set(action, handler);
if (!spec) {
actionHandlers.set(action, handler as DeliveryActionHandler);
return;
}
guardedActions.set(action, { spec, handler: handler as GuardedDeliveryHandler });
actionHandlers.set(action, (content, session) => runGuardedDeliveryAction(action, content, session, null));
}
async function runGuardedDeliveryAction(
action: string,
content: Record<string, unknown>,
session: Session,
grant: PendingApproval | null,
): Promise<void> {
const entry = guardedActions.get(action);
if (!entry) {
log.warn('Unknown guarded delivery action', { action });
return;
}
const { spec, handler } = entry;
if (spec.precheck && !(await spec.precheck(content, session))) return;
const decision = guard({
action: spec.guardAction,
actor: { kind: 'agent', agentGroupId: session.agent_group_id, sessionId: session.id },
payload: content,
grant,
});
if (decision.effect === 'deny') {
log.warn('Delivery action denied by guard', { action, reason: decision.reason });
spec.onDeny?.(content, session, decision.reason);
return;
}
if (decision.effect === 'hold') {
await spec.requestHold(content, session);
return;
}
await handler(content, session);
}
/**
* Approve continuation for a guard-wrapped delivery action: re-enter the
* wrapped entry with the approval row as the grant. The guard treats the
* grant as hold-satisfied but re-runs the structural baseline, so
* approve-then-revoke does not execute. Domains register this as their
* approval handler in the same line that registers the action.
*/
export function reenterGuardedDeliveryAction(action: string) {
return async (ctx: { session: Session | null; payload: Record<string, unknown>; approval: PendingApproval }) => {
if (!ctx.session) {
log.warn('Guarded delivery action approval resolved without a session — dropping', { action });
return;
}
await runGuardedDeliveryAction(action, ctx.payload, ctx.session, ctx.approval);
};
}
/** Look up a registered delivery-action handler. Lets module registrations be behavior-tested. */
@@ -422,6 +517,13 @@ export function getDeliveryAction(action: string): DeliveryActionHandler | undef
return actionHandlers.get(action);
}
/** Registered delivery actions with their guard mapping, for the conformance test. */
export function listDeliveryActions(): { action: string; guardAction: string | null }[] {
return [...actionHandlers.keys()]
.sort()
.map((action) => ({ action, guardAction: guardedActions.get(action)?.spec.guardAction ?? null }));
}
/**
* Handle system actions from the container agent.
* These are written to messages_out because the container can't write to inbound.db.
+106
View File
@@ -0,0 +1,106 @@
/**
* Guard conformance — the non-bypass invariant, shared by CI and boot.
*
* CI only protects code that goes through the repo's CI, but NanoClaw's
* extension model is skill-installed code: /add-* skills copy modules into
* the user's tree and register handlers on machines where the test suite
* never runs. Running the same walk at boot turns the conformance test's
* guarantee into a runtime invariant at exactly that trust boundary: every
* registration is an import-time side effect, so by the time main() runs
* the registries are complete and an unmapped privileged entry is
* detectable before the host accepts a single message.
*
* Fail-closed: the host refuses to start (the upgrade-tripwire posture).
* A conformance failure is code mis-composition — fixable with the host
* down — and it surfaces at skill-install time, when the installing agent
* is watching, instead of running unguarded until someone runs pnpm test.
*
* Limit: the walk verifies DECLARED mappings ("every delivery action is
* guarded or explicitly exempt") — it cannot infer which actions are
* privileged. That declaration stays on the author; the exemption list
* below is the loud, reviewable escape hatch.
*/
import { commandGuardAction } from './cli/guard.js';
import { listCommands } from './cli/registry.js';
import { listDeliveryActions } from './delivery.js';
import { getGuardedAction } from './guard/index.js';
import { log } from './log.js';
/**
* Delivery actions that deliberately carry no guard mapping (the declared
* exemption class, per the guarded-actions design decision 1):
* - scheduling self-actions — an agent mutating only its own task rows;
* not a privileged action class (yet).
* - cli_request — the transport bridge into dispatch(); every inner
* command is guarded at dispatch, so the envelope carries no privilege.
*/
export const EXEMPT_DELIVERY_ACTIONS = new Set([
'schedule_task',
'cancel_task',
'pause_task',
'resume_task',
'update_task',
'cli_request',
]);
/** Walk the live registries against the guard catalog. Empty = conformant. */
export function guardConformanceViolations(): string[] {
const violations: string[] = [];
for (const cmd of listCommands()) {
const entry = getGuardedAction(commandGuardAction(cmd));
if (!entry) {
violations.push(`command "${cmd.name}" has no guard-catalog entry`);
continue;
}
if (cmd.access === 'approval' && entry.approvalAction !== 'cli_command') {
violations.push(`mutating command "${cmd.name}" maps to a catalog entry that cannot hold`);
}
}
for (const { action, guardAction } of listDeliveryActions()) {
if (guardAction === null) {
if (!EXEMPT_DELIVERY_ACTIONS.has(action)) {
violations.push(`delivery action "${action}" is neither guard-mapped nor on the declared exemption list`);
}
continue;
}
if (!getGuardedAction(guardAction)) {
violations.push(`delivery action "${action}" maps to unregistered guard action "${guardAction}"`);
}
}
return violations;
}
/**
* Boot check: refuse to start when any privileged registration is unmapped.
* Call after all import-time registrations (any point in main()).
*/
export function enforceGuardConformance(): void {
const violations = guardConformanceViolations();
if (violations.length === 0) return;
console.error(
[
'',
'='.repeat(64),
'NanoClaw stopped: guard conformance failure',
'='.repeat(64),
'A privileged registration is not mapped to the guard catalog —',
'it would run with no allow/hold/deny decision. This usually means',
'a skill (or local change) registered a command or delivery action',
'without a guard spec.',
'',
...violations.map((v) => ` - ${v}`),
'',
'Fix the registration (pass a guard spec / derive a catalog entry),',
'or — only for genuinely unprivileged self-actions — add it to',
'EXEMPT_DELIVERY_ACTIONS in src/guard-conformance.ts.',
'='.repeat(64),
'',
].join('\n'),
);
log.error('Guard conformance failure — refusing to start', { violations });
process.exit(1);
}
+53
View File
@@ -0,0 +1,53 @@
/**
* The action catalog — the enforcement boundary.
*
* An action either is in the catalog (and passes a decision) or is not (and
* needs none — reads, scheduling self-actions). Declaration is registration:
* entries are derived at the registries' registration sites (command
* registry, delivery actions, response handlers, interceptors, module
* edges), never maintained in a second file. The conformance test walks the
* registries against this catalog so an unmapped privileged action fails CI.
*/
import { log } from '../log.js';
import type { GuardDecision, GuardInput } from './types.js';
import type { PendingApproval } from '../types.js';
export interface GuardedActionSpec {
/** Dotted action name — the catalog key. */
action: string;
/**
* Today's structural checks for this action, verbatim — the only source of
* allow. Runs on every consult, including approved replays (a grant
* satisfies a hold, never a deny).
*/
baseline: (input: GuardInput) => GuardDecision;
/**
* The pending_approvals.action its holds resolve through — a grant is only
* accepted when its row carries this action. Omit for actions that can
* never be held (deny/allow-only baselines).
*/
approvalAction?: string;
/**
* Extra domain binding between a grant and the replayed input (e.g. the
* a2a target must match the held message). Runs in addition to the
* approvalAction + live-row checks.
*/
grantMatches?: (grant: PendingApproval, input: GuardInput) => boolean;
}
const catalog = new Map<string, GuardedActionSpec>();
export function registerGuardedAction(spec: GuardedActionSpec): void {
if (catalog.has(spec.action)) {
log.warn('Guarded action re-registered (overwriting)', { action: spec.action });
}
catalog.set(spec.action, spec);
}
export function getGuardedAction(action: string): GuardedActionSpec | undefined {
return catalog.get(action);
}
export function listGuardedActions(): GuardedActionSpec[] {
return [...catalog.values()].sort((a, b) => a.action.localeCompare(b.action));
}
+89
View File
@@ -0,0 +1,89 @@
/**
* Guard conformance — the non-bypass invariant, checked structurally.
*
* Walks the real command registry and the real delivery-action registry
* (loaded via their production barrels) against the guard catalog. The walk
* itself lives in src/guard-conformance.ts and runs twice: here in CI, and
* at every boot (enforceGuardConformance in index.ts refuses to start on a
* violation) — CI can't see skill-installed registrations, the boot check
* can. A new privileged command or delivery action cannot quietly ship
* ungated: registration derives the catalog entry, and this walk makes
* drift loud.
*
* The declared exemption classes live with the walk
* (EXEMPT_DELIVERY_ACTIONS): scheduling self-actions and the cli_request
* transport bridge (its inner commands are guarded at dispatch). Reads
* (list/get/help) are catalog-mapped via registration too, but their
* baselines allow; the mutating set is what MUST be mapped.
*/
import { describe, expect, it } from 'vitest';
// Production barrels — side-effect imports populate the real registries.
import '../cli/commands/index.js';
import '../modules/index.js';
import '../cli/delivery-action.js';
import { listCommands } from '../cli/registry.js';
import { commandGuardAction } from '../cli/guard.js';
import { listDeliveryActions, registerDeliveryAction } from '../delivery.js';
import { EXEMPT_DELIVERY_ACTIONS, guardConformanceViolations } from '../guard-conformance.js';
import { getGuardedAction } from './catalog.js';
describe('guard conformance', () => {
it('the full walk (shared with the boot check) reports zero violations', () => {
expect(guardConformanceViolations()).toEqual([]);
});
it('every mutating ncl command maps to a guard catalog entry that can hold', () => {
const mutating = listCommands().filter((cmd) => cmd.access === 'approval');
expect(mutating.length).toBeGreaterThan(0);
const unmapped = mutating.filter((cmd) => {
const entry = getGuardedAction(commandGuardAction(cmd));
return !entry || entry.approvalAction !== 'cli_command';
});
expect(unmapped.map((c) => c.name)).toEqual([]);
});
it('every registered command (reads included) has a catalog entry — denied reads still surface as denials', () => {
const unmapped = listCommands().filter((cmd) => !getGuardedAction(commandGuardAction(cmd)));
expect(unmapped.map((c) => c.name)).toEqual([]);
});
it('every delivery action is guard-mapped or on the declared exemption list', () => {
const actions = listDeliveryActions();
expect(actions.length).toBeGreaterThan(0);
const unmapped = actions.filter(
({ action, guardAction }) => guardAction === null && !EXEMPT_DELIVERY_ACTIONS.has(action),
);
expect(unmapped.map((a) => a.action)).toEqual([]);
const danglingCatalog = actions.filter(({ guardAction }) => guardAction !== null && !getGuardedAction(guardAction));
expect(danglingCatalog.map((a) => a.action)).toEqual([]);
});
it('the privileged delivery actions are the guarded ones', () => {
const guarded = Object.fromEntries(
listDeliveryActions()
.filter((a) => a.guardAction !== null)
.map((a) => [a.action, a.guardAction]),
);
expect(guarded).toEqual({
create_agent: 'agents.create',
install_packages: 'self_mod.install_packages',
add_mcp_server: 'self_mod.add_mcp_server',
});
});
// KEEP LAST: registers a rogue action into the shared per-worker registry,
// so every walk after this point sees the violation.
it('the walk names an unguarded, non-exempt delivery action (what the boot check refuses on)', () => {
registerDeliveryAction('test_rogue_privileged_action', async () => {});
const violations = guardConformanceViolations();
expect(violations).toHaveLength(1);
expect(violations[0]).toContain('test_rogue_privileged_action');
expect(violations[0]).toContain('neither guard-mapped nor on the declared exemption list');
});
});
+145
View File
@@ -0,0 +1,145 @@
/**
* Guard decision-function unit tests: the baseline is the decision (allow /
* hold / deny returned as-is, non-catalog actions allow), grant semantics
* (satisfies holds, never denies; invalid → refuse), and the fail-closed
* posture on a throwing baseline.
*
* Uses synthetic catalog actions registered per test — the registry is
* per-worker module state with no reset, so action names are unique.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { ApproverRule } from '../types.js';
import { guard } from './guard.js';
import { registerGuardedAction } from './catalog.js';
import { ALLOW, DENY, HOLD, type GuardInput } from './types.js';
const mockGetPendingApproval = vi.fn();
vi.mock('../db/sessions.js', () => ({
getPendingApproval: (...args: unknown[]) => mockGetPendingApproval(...args),
}));
vi.mock('../log.js', () => ({
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
}));
const AGENT = { kind: 'agent', agentGroupId: 'ag-1', sessionId: 'sess-1' } as const;
function input(action: string, extra: Partial<GuardInput> = {}): GuardInput {
return { action, actor: AGENT, payload: {}, ...extra };
}
const AOS_G1: ApproverRule = { kind: 'admins-of-scope', agentGroupId: 'ag-1', deliveredTo: null };
beforeEach(() => {
mockGetPendingApproval.mockReset();
});
afterEach(() => {
vi.clearAllMocks();
});
describe('the baseline is the decision', () => {
it('non-catalog action → allow', () => {
expect(guard(input('test.unregistered-read')).effect).toBe('allow');
});
it('baseline allow → allow', () => {
registerGuardedAction({ action: 't.allow1', baseline: () => ALLOW('ok') });
expect(guard(input('t.allow1')).effect).toBe('allow');
});
it('baseline hold → hold, carrying the approver rule and scope', () => {
registerGuardedAction({ action: 't.hold1', baseline: () => HOLD(AOS_G1, 'global', 'needs approval') });
const d = guard(input('t.hold1'));
expect(d.effect).toBe('hold');
if (d.effect === 'hold') {
expect(d.approverRule).toEqual(AOS_G1);
expect(d.approverScope).toBe('global');
}
});
it('baseline deny → deny, carrying the reason', () => {
registerGuardedAction({ action: 't.deny1', baseline: () => DENY('structurally unauthorized') });
const d = guard(input('t.deny1'));
expect(d.effect).toBe('deny');
if (d.effect === 'deny') expect(d.reason).toBe('structurally unauthorized');
});
});
describe('grants', () => {
const grantRow = (action: string) =>
({ approval_id: 'appr-1', action, payload: '{}' }) as unknown as NonNullable<GuardInput['grant']>;
it('a valid live grant satisfies a hold', () => {
registerGuardedAction({
action: 't.g1',
approvalAction: 'g1_approved',
baseline: () => HOLD(AOS_G1, 'group', 'b'),
});
const grant = grantRow('g1_approved');
mockGetPendingApproval.mockReturnValue(grant);
expect(guard(input('t.g1', { grant })).effect).toBe('allow');
});
it('a grant never satisfies a deny — the baseline is re-checked live', () => {
registerGuardedAction({ action: 't.g2', approvalAction: 'g2_approved', baseline: () => DENY('revoked since') });
const grant = grantRow('g2_approved');
mockGetPendingApproval.mockReturnValue(grant);
const d = guard(input('t.g2', { grant }));
expect(d.effect).toBe('deny');
if (d.effect === 'deny') expect(d.reason).toBe('revoked since');
});
it('a dead grant (row deleted) refuses instead of re-holding', () => {
registerGuardedAction({
action: 't.g3',
approvalAction: 'g3_approved',
baseline: () => HOLD(AOS_G1, 'group', 'b'),
});
mockGetPendingApproval.mockReturnValue(undefined);
const d = guard(input('t.g3', { grant: grantRow('g3_approved') }));
expect(d.effect).toBe('deny');
});
it("a grant for a different action doesn't transfer", () => {
registerGuardedAction({
action: 't.g4',
approvalAction: 'g4_approved',
baseline: () => HOLD(AOS_G1, 'group', 'b'),
});
const grant = grantRow('other_action');
mockGetPendingApproval.mockReturnValue(grant);
expect(guard(input('t.g4', { grant })).effect).toBe('deny');
});
it('a domain grantMatches binding can refuse a payload mismatch', () => {
registerGuardedAction({
action: 't.g5',
approvalAction: 'g5_approved',
grantMatches: () => false,
baseline: () => HOLD(AOS_G1, 'group', 'b'),
});
const grant = grantRow('g5_approved');
mockGetPendingApproval.mockReturnValue(grant);
expect(guard(input('t.g5', { grant })).effect).toBe('deny');
});
it('a grant on an already-allowed action is a no-op', () => {
registerGuardedAction({ action: 't.g6', approvalAction: 'g6_approved', baseline: () => ALLOW('ok') });
const grant = grantRow('g6_approved');
mockGetPendingApproval.mockReturnValue(grant);
expect(guard(input('t.g6', { grant })).effect).toBe('allow');
});
});
describe('fail-closed posture', () => {
it('a throwing baseline denies', () => {
registerGuardedAction({
action: 't.f1',
baseline: () => {
throw new Error('boom');
},
});
expect(guard(input('t.f1')).effect).toBe('deny');
});
});
+65
View File
@@ -0,0 +1,65 @@
/**
* guard() — the one decision function every privileged action consults.
*
* The decision is the catalog entry's structural baseline — today's code
* checks, registered per action at the module edges. Non-catalog actions
* (reads, scheduling self-actions) allow. Policy-as-data (tighten-only rule
* sources composing with the baseline) is deliberately deferred to phase 3
* of the guarded-actions design, where the generalized rules table arrives
* with its first operator-visible consumer; until then the one policy table
* (agent_message_policies) is consulted inside the a2a.send baseline.
*
* Grants: an approved replay carries the verified approval row. A valid
* grant (live pending row whose action matches the catalog entry's approval
* action, plus any domain binding) satisfies a hold — the human already
* decided — but NEVER a deny: the baseline is re-checked live, so
* approve-then-revoke no longer executes. A grant that is present but
* invalid fails closed to deny (no second card).
*
* The guard itself fails closed: a throwing baseline denies.
*/
import { getPendingApproval } from '../db/sessions.js';
import { log } from '../log.js';
import { getGuardedAction } from './catalog.js';
import { ALLOW, DENY, type GuardDecision, type GuardInput } from './types.js';
export function guard(input: GuardInput): GuardDecision {
const entry = getGuardedAction(input.action);
let decision: GuardDecision;
try {
decision = entry ? entry.baseline(input) : ALLOW('non-catalog action');
} catch (err) {
log.error('Guard evaluation threw — failing closed', { action: input.action, err });
return DENY('guard failure (failing closed)');
}
if (!input.grant || decision.effect !== 'hold') {
// A grant never loosens a deny (the baseline re-check is live), and a
// grant on an already-allowed action is a no-op.
return decision;
}
// An invalid grant on a replay is a refusal, not a fresh hold — approved
// replays must execute exactly once.
if (entry && grantSatisfies(input, entry.approvalAction, entry.grantMatches)) {
return ALLOW(`hold satisfied by approval ${input.grant.approval_id}`);
}
return DENY('replay carried an invalid or mismatched grant');
}
function grantSatisfies(
input: GuardInput,
approvalAction: string | undefined,
grantMatches: ((grant: NonNullable<GuardInput['grant']>, input: GuardInput) => boolean) | undefined,
): boolean {
const grant = input.grant;
if (!grant || !approvalAction) return false;
if (grant.action !== approvalAction) return false;
// The row must still be live — resolution deletes it, so a grant can only
// execute once and a fabricated row object doesn't pass.
const live = getPendingApproval(grant.approval_id);
if (!live || live.action !== approvalAction) return false;
if (grantMatches && !grantMatches(grant, input)) return false;
return true;
}
+10
View File
@@ -0,0 +1,10 @@
/**
* Guard — the privileged-action decision seam (guarded-actions phase 2).
*
* See the guarded-actions decisions doc on the team hub. One decision
* function (guard.ts) and a registration-derived action catalog (catalog.ts).
* Domain-free leaf: domain baselines register from the domain modules' edges.
*/
export { guard } from './guard.js';
export { registerGuardedAction, getGuardedAction, listGuardedActions, type GuardedActionSpec } from './catalog.js';
export { ALLOW, DENY, HOLD, type GuardActor, type GuardDecision, type GuardInput } from './types.js';
+47
View File
@@ -0,0 +1,47 @@
/**
* Guard vocabulary — the decision seam every privileged action passes.
*
* The guard is a domain-free leaf: this module may import the DB read layer,
* config, log, and shared types — never src/cli/* or src/modules/*. Domain
* knowledge (what an action's structural baseline checks) arrives via
* registration: catalog entries (catalog.ts) are registered by the domain
* modules at their module edges.
*/
import type { ApproverRule, ApproverScope, PendingApproval } from '../types.js';
/** Who is attempting the action. Mirrors the CLI CallerContext + click identities. */
export type GuardActor =
| { kind: 'host' }
| { kind: 'agent'; agentGroupId: string; sessionId?: string }
| { kind: 'human'; userId: string }
| { kind: 'system' };
export interface GuardInput {
/** Dotted catalog action name, e.g. 'roles.grant', 'agents.create', 'a2a.send'. */
action: string;
actor: GuardActor;
/** Domain resource reference, e.g. { from, to } for a2a.send. */
resource?: Record<string, string>;
/** Action arguments — what the card summarizes and rules may later match on. */
payload: Record<string, unknown>;
/**
* Verified approval row carried by an approved replay. A valid grant
* satisfies a hold (the human already decided) but never a deny — the
* structural baseline is re-checked live on every replay.
*/
grant?: PendingApproval | null;
}
export type GuardDecision =
| { effect: 'allow'; reason: string }
| { effect: 'hold'; approverRule: ApproverRule; approverScope: ApproverScope; reason: string }
| { effect: 'deny'; reason: string };
export const ALLOW = (reason: string): GuardDecision => ({ effect: 'allow', reason });
export const DENY = (reason: string): GuardDecision => ({ effect: 'deny', reason });
export const HOLD = (approverRule: ApproverRule, approverScope: ApproverScope, reason: string): GuardDecision => ({
effect: 'hold',
approverRule,
approverScope,
reason,
});
+7
View File
@@ -14,6 +14,7 @@ import { initDb } from './db/connection.js';
import { runMigrations } from './db/migrations/index.js';
import { ensureContainerRuntimeRunning, cleanupOrphans } from './container-runtime.js';
import { startActiveDeliveryPoll, startSweepDeliveryPoll, setDeliveryAdapter, stopDeliveryPolls } from './delivery.js';
import { enforceGuardConformance } from './guard-conformance.js';
import { startHostSweep, stopHostSweep } from './host-sweep.js';
import { routeInbound } from './router.js';
import { log } from './log.js';
@@ -69,6 +70,12 @@ async function main(): Promise<void> {
// outside the sanctioned path (raw `git pull` instead of /update-nanoclaw).
enforceUpgradeTripwire();
// 0.6 Guard conformance — every import-time registration has already run;
// refuse to start if any privileged command / delivery action is unmapped
// (CI can't see skill-installed code — this makes the invariant hold at
// the boundary where third-party registrations enter).
enforceGuardConformance();
// 1. Init central DB
const dbPath = path.join(DATA_DIR, 'v2.db');
const db = initDb(dbPath);
+56 -42
View File
@@ -27,14 +27,15 @@ import { getAgentGroup } from '../../db/agent-groups.js';
import { getInboundSourceSessionId, getMostRecentPeerSourceSessionId } from '../../db/session-db.js';
import { getSession } from '../../db/sessions.js';
import { wakeContainer } from '../../container-runner.js';
import { guard } from '../../guard/index.js';
import { log } from '../../log.js';
import { openInboundDb, resolveSession, sessionDir, writeSessionMessage } from '../../session-manager.js';
import type { Session } from '../../types.js';
import type { PendingApproval, Session } from '../../types.js';
import { requestApproval } from '../approvals/index.js';
import { hasDestination } from './db/agent-destinations.js';
import { getMessagePolicy } from './db/agent-message-policies.js';
import { A2A_MESSAGE_GATE_ACTION } from './guard.js';
export { isSafeAttachmentName };
export { A2A_MESSAGE_GATE_ACTION } from './guard.js';
export interface ForwardedAttachment {
name: string;
@@ -230,56 +231,67 @@ function resolveTargetSession(msg: RoutableAgentMessage, sourceSession: Session,
return resolveSession(targetAgentGroupId, null, null, 'agent-shared').session;
}
export async function routeAgentMessage(msg: RoutableAgentMessage, session: Session): Promise<void> {
export async function routeAgentMessage(
msg: RoutableAgentMessage,
session: Session,
opts: { grant?: PendingApproval } = {},
): Promise<void> {
const sourceAgentGroupId = session.agent_group_id;
const targetAgentGroupId = msg.platform_id;
if (!targetAgentGroupId) {
throw new Error(`agent-to-agent message ${msg.id} is missing a target agent group id`);
}
const isSelf = targetAgentGroupId === sourceAgentGroupId;
if (!isSelf && !hasDestination(sourceAgentGroupId, 'agent', targetAgentGroupId)) {
throw new Error(`unauthorized agent-to-agent: ${sourceAgentGroupId} has no destination for ${targetAgentGroupId}`);
}
if (!getAgentGroup(targetAgentGroupId)) {
throw new Error(`target agent group ${targetAgentGroupId} not found for message ${msg.id}`);
// The a2a.send baseline (guard.ts) carries the checks verbatim in their
// original order: self-send allow, destination ACL deny, target-exists
// deny, agent_message_policies hold. An approved replay carries the
// grant — the hold is satisfied but the structure is re-checked live, so
// revoking a destination between hold and approve blocks delivery (D3).
const decision = guard({
action: 'a2a.send',
actor: { kind: 'agent', agentGroupId: sourceAgentGroupId, sessionId: session.id },
resource: { from: sourceAgentGroupId, to: targetAgentGroupId },
payload: { id: msg.id, platform_id: targetAgentGroupId, content: msg.content, in_reply_to: msg.in_reply_to },
grant: opts.grant ?? null,
});
if (decision.effect === 'deny') {
throw new Error(decision.reason);
}
// Gated edge: hold the message and return (not throw) so the delivery loop
// consumes the outbound row; `applyA2aMessageGate` re-routes it on approve.
if (!isSelf) {
const policy = getMessagePolicy(sourceAgentGroupId, targetAgentGroupId);
if (policy) {
const { approver } = policy;
const sourceName = getAgentGroup(sourceAgentGroupId)?.name ?? sourceAgentGroupId;
const targetName = getAgentGroup(targetAgentGroupId)?.name ?? targetAgentGroupId;
await requestApproval({
session,
agentName: sourceName,
action: A2A_MESSAGE_GATE_ACTION,
approverUserId: approver,
title: 'Message approval',
question: buildGateQuestion(sourceName, targetName, msg.content),
payload: {
id: msg.id,
platform_id: targetAgentGroupId,
content: msg.content,
in_reply_to: msg.in_reply_to,
},
});
log.info('Agent message held for approval', {
from: sourceAgentGroupId,
to: targetAgentGroupId,
msgId: msg.id,
});
return;
}
// consumes the outbound row; `applyA2aMessageGate` re-enters here with the
// grant on approve.
if (decision.effect === 'hold') {
const approver = decision.approverRule.kind === 'exclusive' ? decision.approverRule.approverUserId : undefined;
const sourceName = getAgentGroup(sourceAgentGroupId)?.name ?? sourceAgentGroupId;
const targetName = getAgentGroup(targetAgentGroupId)?.name ?? targetAgentGroupId;
await requestApproval({
session,
agentName: sourceName,
action: A2A_MESSAGE_GATE_ACTION,
approverUserId: approver,
approverScope: decision.approverScope,
title: 'Message approval',
question: buildGateQuestion(sourceName, targetName, msg.content),
payload: {
id: msg.id,
platform_id: targetAgentGroupId,
content: msg.content,
in_reply_to: msg.in_reply_to,
},
});
log.info('Agent message held for approval', {
from: sourceAgentGroupId,
to: targetAgentGroupId,
msgId: msg.id,
});
return;
}
await performAgentRoute(msg, session, targetAgentGroupId);
}
export const A2A_MESSAGE_GATE_ACTION = 'a2a_message_gate';
const GATE_CARD_BODY_MAX = 1500;
function parseMessageContent(contentStr: string): { text: string; files: string[] } {
@@ -308,9 +320,11 @@ function buildGateQuestion(sourceName: string, targetName: string, contentStr: s
/**
* Cross-session route: pick the target session, forward files, write to its
* inbound DB, wake it. Authorization is the caller's responsibility.
* inbound DB, wake it. Module-private the only door is routeAgentMessage's
* guard decision (the approve continuation re-enters with a grant rather
* than calling this directly).
*/
export async function performAgentRoute(
async function performAgentRoute(
msg: RoutableAgentMessage,
session: Session,
targetAgentGroupId: string,
+140 -21
View File
@@ -2,26 +2,48 @@
* Tests for create_agent host-side authorization.
*
* Regression guard for the audit finding: `create_agent` is a privileged
* central-DB write with no host-side authz. The fix authorizes by CLI scope
* trusted owner agent groups ('global') create directly; confined groups
* ('group', the default and the prompt-injection victim) must get admin
* approval. These tests pin that branch decision.
* central-DB write with no host-side authz. Authorization is the guard's
* `agents.create` baseline trusted owner agent groups ('global') create
* directly; confined groups ('group', the default and the prompt-injection
* victim) hold for admin approval. These tests drive the REAL wrapped
* delivery action (the only reachable path) and the approve continuation's
* grant-carrying re-entry.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { Session } from '../../types.js';
import type { PendingApproval, Session } from '../../types.js';
// Mocks for the collaborators the branch decides between / depends on.
const mockRequestApproval = vi.fn().mockResolvedValue(undefined);
const mockGetContainerConfig = vi.fn();
const mockCreateAgentGroup = vi.fn();
const mockInitGroupFilesystem = vi.fn();
const mockUpdateScalars = vi.fn();
const mockWriteDestinations = vi.fn();
const mockNotifyWrite = vi.fn();
// vi.hoisted: the module barrel import below runs before this file's const
// initializers, and the mock factories close over this state.
const {
mockRequestApproval,
mockGetContainerConfig,
mockCreateAgentGroup,
mockInitGroupFilesystem,
mockUpdateScalars,
mockWriteDestinations,
mockNotifyWrite,
liveApprovals,
approvalHandlers,
} = vi.hoisted(() => ({
mockRequestApproval: vi.fn().mockResolvedValue(undefined),
mockGetContainerConfig: vi.fn(),
mockCreateAgentGroup: vi.fn(),
mockInitGroupFilesystem: vi.fn(),
mockUpdateScalars: vi.fn(),
mockWriteDestinations: vi.fn(),
mockNotifyWrite: vi.fn(),
liveApprovals: new Map<string, import('../../types.js').PendingApproval>(),
approvalHandlers: new Map<string, (ctx: Record<string, unknown>) => Promise<void>>(),
}));
vi.mock('../approvals/index.js', () => ({
requestApproval: (...a: unknown[]) => mockRequestApproval(...a),
notifyAgent: vi.fn(),
registerApprovalHandler: (action: string, handler: (ctx: Record<string, unknown>) => Promise<void>) => {
approvalHandlers.set(action, handler);
},
}));
vi.mock('../../db/container-configs.js', () => ({
getContainerConfig: (...a: unknown[]) => mockGetContainerConfig(...a),
@@ -42,36 +64,84 @@ vi.mock('./write-destinations.js', () => ({
vi.mock('./db/agent-destinations.js', () => ({
getDestinationByName: () => undefined,
createDestination: vi.fn(),
hasDestination: () => true,
normalizeName: (s: string) => s.toLowerCase().replace(/[^a-z0-9]+/g, '-'),
}));
// notifyAgent writes to the session inbound.db + wakes the container; stub both.
// delivery.ts and agent-route.ts pull more session-manager exports at import time.
vi.mock('../../session-manager.js', () => ({
writeSessionMessage: (...a: unknown[]) => mockNotifyWrite(...a),
openInboundDb: vi.fn(),
openOutboundDb: vi.fn(),
clearOutbox: vi.fn(),
readOutboxFiles: vi.fn().mockReturnValue([]),
resolveSession: vi.fn(),
sessionDir: vi.fn().mockReturnValue('/tmp/nowhere'),
inboundDbPath: vi.fn().mockReturnValue('/tmp/nowhere/inbound.db'),
}));
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
}));
vi.mock('../../db/sessions.js', () => ({
getSession: (id: string) => ({ id, agent_group_id: 'ag-1' }),
getPendingApproval: (id: string) => liveApprovals.get(id),
getRunningSessions: () => [],
getActiveSessions: () => [],
createPendingQuestion: vi.fn(),
}));
import { handleCreateAgent } from './create-agent.js';
// The a2a module barrel registers ./guard.js (catalog entries) and the
// guard-wrapped create_agent delivery action — the path under test.
import './index.js';
import { getDeliveryAction } from '../../delivery.js';
const SESSION = { id: 'sess-1', agent_group_id: 'ag-1' } as Session;
async function runCreateAgent(content: Record<string, unknown>): Promise<void> {
const wrapped = getDeliveryAction('create_agent');
expect(wrapped).toBeDefined();
await wrapped!(content, SESSION, undefined as never);
}
function liveGrant(approvalId: string, payload: Record<string, unknown>): PendingApproval {
const row = {
approval_id: approvalId,
session_id: SESSION.id,
request_id: approvalId,
action: 'create_agent',
payload: JSON.stringify(payload),
created_at: new Date().toISOString(),
agent_group_id: 'ag-1',
channel_type: null,
platform_id: null,
platform_message_id: null,
expires_at: null,
status: 'pending',
title: '',
options_json: '[]',
approver_user_id: null,
approver_rule: 'admins-of-scope',
approver_scope: 'group',
dedup_key: null,
} as PendingApproval;
liveApprovals.set(approvalId, row);
return row;
}
beforeEach(() => {
vi.clearAllMocks();
liveApprovals.clear();
});
afterEach(() => {
vi.restoreAllMocks();
});
describe('handleCreateAgent — scope-based authorization', () => {
describe('create_agent — guard-based authorization (wrapped delivery action)', () => {
it('global scope: creates directly, no approval requested', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' });
await handleCreateAgent({ name: 'Scout', instructions: 'help' }, SESSION);
await runCreateAgent({ name: 'Scout', instructions: 'help' });
expect(mockRequestApproval).not.toHaveBeenCalled();
expect(mockCreateAgentGroup).toHaveBeenCalledTimes(1);
@@ -84,7 +154,7 @@ describe('handleCreateAgent — scope-based authorization', () => {
// dropping the inheritance leaves the child provider-less (→ claude).
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global', provider: 'codex' });
await handleCreateAgent({ name: 'Scout', instructions: 'help' }, SESSION);
await runCreateAgent({ name: 'Scout', instructions: 'help' });
expect(mockInitGroupFilesystem).toHaveBeenCalledWith(
expect.anything(),
@@ -96,7 +166,7 @@ describe('handleCreateAgent — scope-based authorization', () => {
it('claude creator leaves the child provider unset (built-in default)', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' }); // no provider
await handleCreateAgent({ name: 'Scout', instructions: 'help' }, SESSION);
await runCreateAgent({ name: 'Scout', instructions: 'help' });
expect(mockUpdateScalars).not.toHaveBeenCalled();
});
@@ -104,7 +174,7 @@ describe('handleCreateAgent — scope-based authorization', () => {
it('group scope (default): requires approval, does NOT create directly', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
await handleCreateAgent({ name: 'Scout', instructions: 'help' }, SESSION);
await runCreateAgent({ name: 'Scout', instructions: 'help' });
expect(mockRequestApproval).toHaveBeenCalledTimes(1);
expect(mockRequestApproval.mock.calls[0][0]).toMatchObject({ action: 'create_agent' });
@@ -115,7 +185,7 @@ describe('handleCreateAgent — scope-based authorization', () => {
it('missing config: fails closed to approval (no direct create)', async () => {
mockGetContainerConfig.mockReturnValue(undefined);
await handleCreateAgent({ name: 'Scout' }, SESSION);
await runCreateAgent({ name: 'Scout' });
expect(mockRequestApproval).toHaveBeenCalledTimes(1);
expect(mockCreateAgentGroup).not.toHaveBeenCalled();
@@ -124,7 +194,7 @@ describe('handleCreateAgent — scope-based authorization', () => {
it('disabled/other scope: requires approval', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'disabled' });
await handleCreateAgent({ name: 'Scout' }, SESSION);
await runCreateAgent({ name: 'Scout' });
expect(mockRequestApproval).toHaveBeenCalledTimes(1);
expect(mockCreateAgentGroup).not.toHaveBeenCalled();
@@ -133,9 +203,58 @@ describe('handleCreateAgent — scope-based authorization', () => {
it('empty name: neither creates nor requests approval', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' });
await handleCreateAgent({ name: '' }, SESSION);
await runCreateAgent({ name: '' });
expect(mockRequestApproval).not.toHaveBeenCalled();
expect(mockCreateAgentGroup).not.toHaveBeenCalled();
});
});
describe('create_agent — approved replay (grant-carrying re-entry)', () => {
it('valid grant executes exactly once — baseline hold is satisfied, create runs', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
const payload = { name: 'Scout', instructions: 'help' };
const approval = liveGrant('appr-ca-1', payload);
const continuation = approvalHandlers.get('create_agent');
expect(continuation).toBeDefined();
await continuation!({ session: SESSION, payload, approval, userId: 'telegram:admin', notify: vi.fn() });
expect(mockCreateAgentGroup).toHaveBeenCalledTimes(1);
expect(mockRequestApproval).not.toHaveBeenCalled(); // no second card
});
it('dead grant (row already resolved) refuses the replay', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
const payload = { name: 'Scout', instructions: 'help' };
const approval = liveGrant('appr-ca-2', payload);
liveApprovals.delete('appr-ca-2'); // resolution consumed the row
await approvalHandlers.get('create_agent')!({
session: SESSION,
payload,
approval,
userId: 'telegram:admin',
notify: vi.fn(),
});
expect(mockCreateAgentGroup).not.toHaveBeenCalled();
expect(mockRequestApproval).not.toHaveBeenCalled(); // refused, not re-held
});
it('mismatched grant (approved for a different name) refuses the replay', async () => {
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
const approval = liveGrant('appr-ca-3', { name: 'OtherAgent' });
await approvalHandlers.get('create_agent')!({
session: SESSION,
payload: { name: 'Scout' },
approval,
userId: 'telegram:admin',
notify: vi.fn(),
});
expect(mockCreateAgentGroup).not.toHaveBeenCalled();
expect(mockRequestApproval).not.toHaveBeenCalled();
});
});
+32 -58
View File
@@ -1,16 +1,18 @@
/**
* `create_agent` delivery-action handler.
* `create_agent` delivery-action bodies.
*
* SECURITY: `create_agent` writes to the CENTRAL DB (agent_groups,
* container_configs, agent_destinations) and scaffolds host filesystem state
* a privileged operation a confined container is otherwise architecturally
* barred from. The container's MCP tool gate is inside the (untrusted)
* container and is trivially bypassed by writing the outbound system row
* directly, so authorization MUST be enforced host-side. Trusted owner agent
* groups (CLI scope 'global') create directly; every other (confined) group
* requires admin approval via `requestApproval` matching `ncl groups create`
* (access: 'approval') and the self-mod actions. `applyCreateAgent` runs the
* creation on approve; `performCreateAgent` is the shared body.
* directly, so authorization MUST be enforced host-side: the delivery
* registry wraps this action with the guard, whose `agents.create` baseline
* (./guard.ts) is the old cli_scope branch verbatim trusted global-scope
* groups allow, everything else (including unknown config, fail-closed)
* holds for admin approval. On approve the continuation re-enters the
* wrapped action with the approval row as its grant and `createAgent` runs.
* `performCreateAgent` is the module-private body.
*/
import path from 'path';
@@ -23,7 +25,7 @@ import { initGroupFilesystem } from '../../group-init.js';
import { log } from '../../log.js';
import { writeSessionMessage } from '../../session-manager.js';
import type { AgentGroup, Session } from '../../types.js';
import { requestApproval, type ApprovalHandler } from '../approvals/index.js';
import { requestApproval } from '../approvals/index.js';
import { createDestination, getDestinationByName, normalizeName } from './db/agent-destinations.js';
import { writeDestinations } from './write-destinations.js';
@@ -43,41 +45,27 @@ function notifyAgent(session: Session, text: string): void {
}
}
/**
* Delivery-action entry.
*
* Authorization depends on the calling group's CLI scope:
* - `global` (set by init-first-agent for trusted owner agent groups):
* create immediately. create_agent is the intended primitive for these
* privileged agents, and an approval tap on every sub-agent spawn would be
* needless friction.
* - anything else (the default `group` scope the realistic
* prompt-injection victim): require an admin to approve before any
* central-DB write. `applyCreateAgent` runs on approve.
* Unknown/missing config fails closed to the approval path.
*/
export async function handleCreateAgent(content: Record<string, unknown>, session: Session): Promise<void> {
/** Guard precheck: malformed requests are answered without ever creating a hold. */
export function validateCreateAgent(content: Record<string, unknown>, session: Session): boolean {
const name = typeof content.name === 'string' ? content.name : '';
const instructions = typeof content.instructions === 'string' ? content.instructions : null;
if (!name) {
notifyAgent(session, 'create_agent failed: name is required.');
return;
return false;
}
const sourceGroup = getAgentGroup(session.agent_group_id);
if (!sourceGroup) {
if (!getAgentGroup(session.agent_group_id)) {
notifyAgent(session, 'create_agent failed: source agent group not found.');
log.warn('create_agent failed: missing source group', { sessionAgentGroup: session.agent_group_id, name });
return;
return false;
}
return true;
}
const cliScope = getContainerConfig(session.agent_group_id)?.cli_scope ?? 'group';
if (cliScope === 'global') {
// Trusted owner agent group — create directly, then notify (+wake) it.
await performCreateAgent(name, instructions, session, sourceGroup, (text) => notifyAgent(session, text));
return;
}
/** Guard hold: card the requesting group's admin chain. */
export async function requestCreateAgentHold(content: Record<string, unknown>, session: Session): Promise<void> {
const name = typeof content.name === 'string' ? content.name : '';
const instructions = typeof content.instructions === 'string' ? content.instructions : null;
const sourceGroup = getAgentGroup(session.agent_group_id);
if (!sourceGroup) return;
await requestApproval({
session,
@@ -89,36 +77,22 @@ export async function handleCreateAgent(content: Record<string, unknown>, sessio
});
}
/**
* Approval handler: performs the creation once an admin approves a request from
* a confined (non-global) agent group. `session` is the requesting parent.
*/
export const applyCreateAgent: ApprovalHandler = async ({ session, payload, notify }) => {
const name = typeof payload.name === 'string' ? payload.name : '';
const instructions = typeof payload.instructions === 'string' ? payload.instructions : null;
if (!name) {
notify('create_agent approved but the request had no name.');
return;
}
/** Guard allow body: performs the creation (fresh global-scope call or approved replay). */
export async function createAgent(content: Record<string, unknown>, session: Session): Promise<void> {
const name = typeof content.name === 'string' ? content.name : '';
const instructions = typeof content.instructions === 'string' ? content.instructions : null;
const sourceGroup = getAgentGroup(session.agent_group_id);
if (!sourceGroup) {
notify('create_agent approved but the source agent group no longer exists.');
log.warn('create_agent apply failed: missing source group', { sessionAgentGroup: session.agent_group_id, name });
return;
}
if (!name || !sourceGroup) return; // precheck already answered the requester
await performCreateAgent(name, instructions, session, sourceGroup, notify);
};
await performCreateAgent(name, instructions, session, sourceGroup, (text) => notifyAgent(session, text));
}
/**
* Core creation: writes the new agent group + bidirectional destinations and
* scaffolds its filesystem, then reports via `notify`. Authorization is the
* CALLER's responsibility (the global-scope shortcut in handleCreateAgent or
* admin approval via applyCreateAgent) never call this from an unauthorized
* path, as it performs privileged central-DB writes a confined container is
* otherwise barred from.
* CALLER's responsibility (the guard's agents.create decision) never call
* this from an unauthorized path, as it performs privileged central-DB
* writes a confined container is otherwise barred from.
*/
async function performCreateAgent(
name: string,
+96
View File
@@ -0,0 +1,96 @@
/**
* Agent-to-agent guard adapter the module's catalog entries, composed at
* the module edge (imported by ./index.ts).
*
* agents.create the cli_scope branch moved verbatim out of
* create-agent.ts: `global` scope creates directly (create_agent is the
* intended primitive for trusted owner agent groups); anything else the
* default `group` scope, and unknown/missing config, fail-closed holds for
* the requesting group's admin chain.
*
* a2a.send the decision moved verbatim out of routeAgentMessage, in its
* original check order: self-sends allow without a destination row; a
* missing destination row denies; a missing target group denies; an
* agent_message_policies row for the (from, to) pair holds exclusively for
* the row's named approver. The ghost-policy edge (policy row with no
* destination row) denies the destination check precedes the policy check,
* exactly today's outcome. Policy rows can only tighten (hold), never allow:
* absence of a row falls through to the structural checks.
*/
import { getAgentGroup } from '../../db/agent-groups.js';
import { getContainerConfig } from '../../db/container-configs.js';
import { ALLOW, DENY, HOLD, registerGuardedAction } from '../../guard/index.js';
import { hasDestination } from './db/agent-destinations.js';
import { getMessagePolicy } from './db/agent-message-policies.js';
/**
* pending_approvals action string for held a2a messages. Lives here (not in
* agent-route.ts) so agent-route can import this adapter loading the
* consult site guarantees its catalog entry is registered without a cycle.
*/
export const A2A_MESSAGE_GATE_ACTION = 'a2a_message_gate';
registerGuardedAction({
action: 'agents.create',
approvalAction: 'create_agent',
// Bind a create_agent grant to the name that was approved.
grantMatches: (grant, input) => {
try {
return (JSON.parse(grant.payload) as { name?: string }).name === input.payload.name;
} catch {
return false;
}
},
baseline: (input) => {
if (input.actor.kind !== 'agent') return DENY('create_agent is a container-originated action.');
const cliScope = getContainerConfig(input.actor.agentGroupId)?.cli_scope ?? 'group';
if (cliScope === 'global') {
// Trusted owner agent group — an approval tap on every sub-agent spawn
// would be needless friction.
return ALLOW('trusted global-scope agent group');
}
// The realistic prompt-injection victim (default `group` scope) — and any
// unknown config value, fail-closed — requires an admin before any
// central-DB write.
return HOLD(
{ kind: 'admins-of-scope', agentGroupId: input.actor.agentGroupId, deliveredTo: null },
'group',
'agent-initiated create_agent requires admin approval',
);
},
});
registerGuardedAction({
action: 'a2a.send',
approvalAction: A2A_MESSAGE_GATE_ACTION,
// Bind an a2a grant to the exact held message target.
grantMatches: (grant, input) => {
try {
return (JSON.parse(grant.payload) as { platform_id?: string }).platform_id === input.resource?.to;
} catch {
return false;
}
},
baseline: (input) => {
if (input.actor.kind !== 'agent') return DENY('agent-to-agent send requires an agent actor');
const from = input.actor.agentGroupId;
const to = input.resource?.to ?? '';
const isSelf = to === from;
if (!isSelf && !hasDestination(from, 'agent', to)) {
return DENY(`unauthorized agent-to-agent: ${from} has no destination for ${to}`);
}
if (!getAgentGroup(to)) {
return DENY(`target agent group ${to} not found for message ${String(input.payload.id)}`);
}
if (isSelf) return ALLOW('self-send');
const policy = getMessagePolicy(from, to);
if (policy) {
return HOLD(
{ kind: 'exclusive', approverUserId: policy.approver },
'group',
`a2a message policy ${from}${to} holds for ${policy.approver}`,
);
}
return ALLOW('destination grant exists');
},
});
+19 -12
View File
@@ -1,13 +1,14 @@
/**
* Agent-to-agent module inter-agent messaging and on-demand agent creation.
*
* Registers one delivery action (`create_agent`) plus its matching approval
* handler `create_agent` writes central-DB state, so confined (non-global)
* groups require admin approval (the delivery action queues the request;
* `applyCreateAgent` runs on approve); trusted global-scope groups create
* directly. The sibling `channel_type === 'agent'` routing path is NOT a system
* action core `delivery.ts` dispatches into `./agent-route.js` via a dynamic
* import when it sees `msg.channel_type === 'agent'`.
* Registers its guard-catalog entries (./guard.js) and one guard-wrapped
* delivery action (`create_agent`) `create_agent` writes central-DB state,
* so the guard's agents.create baseline holds confined (non-global) groups
* for admin approval while trusted global-scope groups create directly; the
* approval handler re-enters the wrapped action carrying the approval row as
* its grant. The sibling `channel_type === 'agent'` routing path is NOT a
* system action core `delivery.ts` dispatches into `./agent-route.js` via
* a dynamic import when it sees `msg.channel_type === 'agent'`.
*
* Host integration points:
* - `src/container-runner.ts::spawnContainer` dynamically imports
@@ -20,13 +21,19 @@
* system action logs "Unknown system action", `channel_type='agent'` messages
* throw because the module isn't installed.
*/
import { registerDeliveryAction } from '../../delivery.js';
import { registerApprovalHandler } from '../approvals/index.js';
import './guard.js';
import { reenterGuardedDeliveryAction, registerDeliveryAction } from '../../delivery.js';
import { notifyAgent, registerApprovalHandler } from '../approvals/index.js';
import { A2A_MESSAGE_GATE_ACTION } from './agent-route.js';
import { applyCreateAgent, handleCreateAgent } from './create-agent.js';
import { createAgent, requestCreateAgentHold, validateCreateAgent } from './create-agent.js';
import { applyA2aMessageGate } from './message-gate.js';
registerDeliveryAction('create_agent', handleCreateAgent);
registerApprovalHandler('create_agent', applyCreateAgent);
registerDeliveryAction('create_agent', createAgent, {
guardAction: 'agents.create',
precheck: validateCreateAgent,
requestHold: requestCreateAgentHold,
onDeny: (_content, session, reason) => notifyAgent(session, `create_agent denied: ${reason}`),
});
registerApprovalHandler('create_agent', reenterGuardedDeliveryAction('create_agent'));
registerApprovalHandler(A2A_MESSAGE_GATE_ACTION, applyA2aMessageGate);
+79 -10
View File
@@ -2,16 +2,17 @@ import Database from 'better-sqlite3';
import fs from 'fs';
import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest';
import './guard.js'; // register the a2a.send catalog entry (incl. the policy hold)
import { routeAgentMessage } from './agent-route.js';
import { createDestination, deleteDestination, deleteAllDestinationsTouching } from './db/agent-destinations.js';
import { getMessagePolicy, removeMessagePolicy, setMessagePolicy } from './db/agent-message-policies.js';
import { applyA2aMessageGate } from './message-gate.js';
import { initTestDb, closeDb, runMigrations, createAgentGroup } from '../../db/index.js';
import { getDb } from '../../db/connection.js';
import { createSession } from '../../db/sessions.js';
import { createPendingApproval, createSession, deletePendingApproval, getPendingApproval } from '../../db/sessions.js';
import { requestApproval } from '../approvals/index.js';
import { initSessionFolder, inboundDbPath } from '../../session-manager.js';
import type { Session } from '../../types.js';
import type { PendingApproval, Session } from '../../types.js';
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
@@ -67,6 +68,24 @@ function makeSession(id: string, agentGroupId: string): Session {
};
}
/** Seed a live a2a hold row (what requestApproval writes) and return it as the grant. */
function seedA2aHold(approvalId: string, payload: Record<string, unknown>): PendingApproval {
createPendingApproval({
approval_id: approvalId,
session_id: 'sess-A',
request_id: approvalId,
action: 'a2a_message_gate',
payload: JSON.stringify(payload),
created_at: now(),
agent_group_id: A,
title: 'Message approval',
options_json: '[]',
approver_user_id: 'telegram:dana',
approver_rule: 'exclusive',
});
return getPendingApproval(approvalId)!;
}
describe('agent message policies', () => {
let SA: Session;
let SB: Session;
@@ -158,21 +177,71 @@ describe('agent message policies', () => {
expect(readInbound(A, SA.id)).toHaveLength(1);
});
// ── approve handler re-routes the held message ──
it('ghost policy (policy row, no destination row) still denies — deny beats the rule hold', async () => {
deleteDestination(A, 'b'); // removes A→B — the destination ACL now denies
setMessagePolicy(A, B, 'telegram:dana', now()); // ...but a stale policy row remains
await expect(
routeAgentMessage({ id: 'ghost', platform_id: B, content: JSON.stringify({ text: 'x' }), in_reply_to: null }, SA),
).rejects.toThrow(/unauthorized agent-to-agent/);
expect(requestApproval).not.toHaveBeenCalled();
expect(readInbound(B, SB.id)).toHaveLength(0);
});
// ── approve handler re-enters the guarded route with the grant ──
it('applyA2aMessageGate delivers the held message to the target (valid grant)', async () => {
setMessagePolicy(A, B, 'telegram:dana', now());
const payload = { id: 'held-1', platform_id: B, content: JSON.stringify({ text: 'approved!' }), in_reply_to: null };
const approval = seedA2aHold('appr-a2a-1', payload);
it('applyA2aMessageGate delivers the held message to the target', async () => {
const notify = vi.fn();
await applyA2aMessageGate({
session: SA,
userId: 'slack:dana',
notify,
payload: { id: 'held-1', platform_id: B, content: JSON.stringify({ text: 'approved!' }), in_reply_to: null },
});
await applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify, payload, approval });
const bRows = readInbound(B, SB.id);
expect(bRows).toHaveLength(1);
expect(JSON.parse(bRows[0].content).text).toBe('approved!');
expect(notify).not.toHaveBeenCalled();
// The hold is satisfied by the grant — no second card.
expect(requestApproval).not.toHaveBeenCalled();
});
it('D3: destination revoked between hold and approve → the approved replay is blocked', async () => {
setMessagePolicy(A, B, 'telegram:dana', now());
const payload = { id: 'held-2', platform_id: B, content: JSON.stringify({ text: 'stale' }), in_reply_to: null };
const approval = seedA2aHold('appr-a2a-2', payload);
deleteDestination(A, 'b'); // revoke A→B while the card is pending
await expect(
applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify: vi.fn(), payload, approval }),
).rejects.toThrow(/unauthorized agent-to-agent/);
expect(readInbound(B, SB.id)).toHaveLength(0);
});
it('mismatched grant (held for another target) refuses the replay', async () => {
setMessagePolicy(A, B, 'telegram:dana', now());
// Grant was approved for a message to A (different target than the replay).
const approval = seedA2aHold('appr-a2a-3', { id: 'other', platform_id: A, content: '{}', in_reply_to: null });
const payload = { id: 'held-3', platform_id: B, content: JSON.stringify({ text: 'swap' }), in_reply_to: null };
await expect(
applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify: vi.fn(), payload, approval }),
).rejects.toThrow(/invalid or mismatched grant/);
expect(readInbound(B, SB.id)).toHaveLength(0);
});
it('a grant only works while its row is live (executes once)', async () => {
setMessagePolicy(A, B, 'telegram:dana', now());
const payload = { id: 'held-4', platform_id: B, content: JSON.stringify({ text: 'once' }), in_reply_to: null };
const approval = seedA2aHold('appr-a2a-4', payload);
deletePendingApproval(approval.approval_id); // resolution already consumed the row
await expect(
applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify: vi.fn(), payload, approval }),
).rejects.toThrow(/invalid or mismatched grant/);
expect(readInbound(B, SB.id)).toHaveLength(0);
});
// ── ghost-gate cleanup ──
+12 -3
View File
@@ -1,9 +1,13 @@
/** Approve handler for a held a2a message. (Reject is handled by the generic response-handler path.) */
import { log } from '../../log.js';
import type { ApprovalHandler } from '../approvals/index.js';
import { performAgentRoute, type RoutableAgentMessage } from './agent-route.js';
import { routeAgentMessage, type RoutableAgentMessage } from './agent-route.js';
export const applyA2aMessageGate: ApprovalHandler = async ({ session, payload, notify }) => {
export const applyA2aMessageGate: ApprovalHandler = async ({ session, payload, approval, 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.');
@@ -18,7 +22,12 @@ export const applyA2aMessageGate: ApprovalHandler = async ({ session, payload, n
in_reply_to: typeof in_reply_to === 'string' ? in_reply_to : null,
};
await performAgentRoute(msg, session, platform_id);
// One replay semantics (D3): re-enter the guarded route carrying the
// approval row as the grant. The policy hold is satisfied, but the
// structural baseline runs live — un-wiring the pair between hold and
// approve now blocks delivery (the throw surfaces via the response
// handler's "approved, but applying it failed" notify).
await routeAgentMessage(msg, session, { grant: approval });
log.info('Held agent message delivered after approval', {
from: session.agent_group_id,
to: platform_id,
@@ -13,12 +13,21 @@ 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 } from '../../db/sessions.js';
import { getDb } from '../../db/connection.js';
import { createMessagingGroup } from '../../db/messaging-groups.js';
import { createSession, createPendingApproval, getPendingApproval, getSession } 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 { handleApprovalsResponse } from './response-handler.js';
import { registerApprovalHandler, registerApprovalResolvedHandler, type ApprovalResolvedEvent } from './primitive.js';
import {
registerApprovalHandler,
registerApprovalRequestedHandler,
registerApprovalResolvedHandler,
requestApproval,
type ApprovalRequestedEvent,
type ApprovalResolvedEvent,
} from './primitive.js';
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
@@ -100,7 +109,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');
});
@@ -150,3 +159,43 @@ describe('approval-resolved callbacks', () => {
expect(events).toEqual(['boom', 'after']);
});
});
describe('approval-requested callbacks', () => {
it('fires when requestApproval creates a hold, with the row and the delivered approver', async () => {
// The approver needs a reachable DM for the delivery walk.
createMessagingGroup({
id: 'mg-dm-admin',
channel_type: 'slack',
platform_id: 'dm-admin',
name: 'Admin DM',
is_group: 0,
unknown_sender_policy: 'public',
created_at: now(),
});
getDb()
.prepare(`INSERT INTO user_dms (user_id, channel_type, messaging_group_id, resolved_at) VALUES (?, ?, ?, ?)`)
.run('slack:admin-1', 'slack', 'mg-dm-admin', now());
const events: ApprovalRequestedEvent[] = [];
registerApprovalRequestedHandler((event) => {
if (event.approval.action === 'test_requested_action') events.push(event);
});
await requestApproval({
session: getSession('sess-1')!,
agentName: 'Agent',
action: 'test_requested_action',
payload: { thing: 1 },
title: 'Test hold',
question: 'Approve the thing?',
});
expect(events).toHaveLength(1);
expect(events[0].deliveredTo).toBe('slack:admin-1');
expect(events[0].session?.id).toBe('sess-1');
expect(events[0].approval.agent_group_id).toBe('ag-1');
expect(events[0].approval.approver_rule).toBe('admins-of-scope');
// The event carries the live row.
expect(getPendingApproval(events[0].approval.approval_id)).toBeDefined();
});
});
+222
View File
@@ -0,0 +1,222 @@
/**
* mayResolve matrix the one click-authorization rule for every hold.
*
* Covers each approver-rule 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 approver 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 { approverRuleOf, mayResolve } from './approver-rule.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-approver-rule' };
});
const TEST_DIR = '/tmp/nanoclaw-test-approver-rule';
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('approverRuleOf maps row columns onto the rule', () => {
const base = { agent_group_id: 'ag-1' };
expect(approverRuleOf({ ...base, approver_rule: 'exclusive', approver_user_id: DELIVEREE })).toEqual({
kind: 'exclusive',
approverUserId: DELIVEREE,
});
expect(approverRuleOf({ ...base, approver_rule: 'admins-of-scope', approver_user_id: DELIVEREE })).toEqual({
kind: 'admins-of-scope',
agentGroupId: 'ag-1',
deliveredTo: DELIVEREE,
});
expect(approverRuleOf({ ...base, approver_rule: '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(approverRuleOf({ ...base, approver_rule: '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();
});
});
+68
View File
@@ -0,0 +1,68 @@
/**
* Approver rules 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 approver
* 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` / (`approver_rule` +
* `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 approver-rule 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
* approver rule.
*
* `mayResolve` replaces the three divergent click-auth copies (approvals
* response handler, sender handler, channel handler) with one function.
*/
import type { ApproverRule, ApproverScope, PendingApproval } from '../../types.js';
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
export type { ApproverRule, ApproverScope } from '../../types.js';
/** May `clickerUserId` (namespaced `<channel>:<handle>`) resolve a hold with this approver rule + scope? */
export function mayResolve(e: ApproverRule, 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 approver rule a `pending_approvals` row encodes. */
export function approverRuleOf(
approval: Pick<PendingApproval, 'approver_rule' | 'approver_user_id' | 'agent_group_id'>,
): ApproverRule {
if (approval.approver_rule === '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.approver_rule === 'exclusive' ? null : approval.approver_user_id,
};
}
+19 -14
View File
@@ -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);
}
+25 -3
View File
@@ -16,15 +16,21 @@
*
* Startup sweep edits any leftover cards from a previous process to
* "Expired (host restarted)" and drops the rows.
*
* Hold creation and every resolution (click / expiry / sweep) announce
* through the shared approval observers (notifyApprovalRequested /
* notifyApprovalResolved) observers see the full OneCLI lifecycle without
* touching this file.
*/
import { OneCLI, type ApprovalRequest, type ManualApprovalHandle } from '@onecli-sh/sdk';
import { pickApprovalDelivery, pickApprover } from './primitive.js';
import { notifyApprovalRequested, 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 +70,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;
}
@@ -195,6 +210,11 @@ async function handleRequest(request: ApprovalRequest): Promise<Decision> {
options_json: JSON.stringify(onecliOptions),
});
const created = getPendingApproval(approvalId);
if (created) {
await notifyApprovalRequested({ approval: created, session: null, deliveredTo: target.userId });
}
// Expiry timer fires just before the gateway's own TTL so our decision lands
// in time to be recorded, even though the HTTP side will already be closing.
const expiresAtMs = new Date(request.expiresAt).getTime();
@@ -222,6 +242,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 +272,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: '' });
}
}
+139 -24
View File
@@ -23,7 +23,12 @@
*/
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,
getPendingApproval,
getPendingApprovalByDedupKey,
getSession,
} from '../../db/sessions.js';
import { getDeliveryAdapter } from '../../delivery.js';
import { wakeContainer } from '../../container-runner.js';
import { log } from '../../log.js';
@@ -57,11 +62,18 @@ 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>;
/**
* The verified approval row the grant an approved continuation carries
* when it re-enters its guarded entry point. Still live here; resolution
* deletes it after the handler returns, so a grant executes exactly once.
*/
approval: PendingApproval;
/** 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 +100,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;
}
@@ -124,6 +142,51 @@ export async function notifyApprovalResolved(event: ApprovalResolvedEvent): Prom
}
}
// ── Approval-requested callbacks ──
// The creation-side sibling of the resolved observer: fires once whenever a
// hold record comes into existence, whichever stack created it —
// requestApproval (cli_command, create_agent, self-mod, a2a, sender
// admission), the OneCLI credential bridge (its own rows, ids and card), and
// channel registration (as a synthesized hold view). Together with
// notifyApprovalResolved this gives observers the full hold lifecycle with
// zero touch points inside the flows.
export interface ApprovalRequestedEvent {
/**
* The created hold. For holds that live outside pending_approvals
* (channel registration) this is a synthesized view of the same shape.
*/
approval: PendingApproval;
/** Requesting agent's session; null for sessionless holds (sender admission, OneCLI, channel registration). */
session: Session | null;
/** Namespaced user ID (`<channel>:<handle>`) of the approver the card was delivered to. */
deliveredTo: string;
}
export type ApprovalRequestedHandler = (event: ApprovalRequestedEvent) => Promise<void> | void;
const approvalRequestedHandlers: ApprovalRequestedHandler[] = [];
export function registerApprovalRequestedHandler(handler: ApprovalRequestedHandler): void {
approvalRequestedHandlers.push(handler);
}
/** Fire every registered approval-requested callback. Called wherever a hold record is created. */
export async function notifyApprovalRequested(event: ApprovalRequestedEvent): Promise<void> {
for (const handler of approvalRequestedHandlers) {
try {
await handler(event);
// eslint-disable-next-line no-catch-all/no-catch-all -- isolation is the contract: one bad callback must not block the hold or other callbacks
} catch (err) {
log.error('Approval-requested handler threw', {
approvalId: event.approval.approval_id,
action: event.approval.action,
err,
});
}
}
}
// ── Approver picking ──
/**
@@ -200,7 +263,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;
/** Approver-rule 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 +280,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 (approver rule '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,38 +315,59 @@ 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),
approver_rule: approverUserId ? 'exclusive' : 'admins-of-scope',
approver_scope: opts.approverScope ?? 'group',
dedup_key: dedupKey ?? null,
});
const created = getPendingApproval(approvalId);
if (created) {
await notifyApprovalRequested({ approval: created, session: session ?? null, deliveredTo: target.userId });
}
const adapter = getDeliveryAdapter();
if (adapter) {
try {
@@ -266,12 +381,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;
}
}
+33 -45
View File
@@ -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 approver rule +
* approver scope (approver-rule.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 { approverRuleOf, mayResolve } from './approver-rule.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(approverRuleOf(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,13 +113,13 @@ 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;
}
const payload = JSON.parse(approval.payload);
try {
await handler({ session, payload, userId, notify });
await handler({ session, payload, approval, userId, notify });
log.info('Approval handled', { approvalId: approval.approval_id, action: approval.action, userId });
} catch (err) {
log.error('Approval handler threw', { approvalId: approval.approval_id, action: approval.action, err });
@@ -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);
}
+143 -31
View File
@@ -54,7 +54,11 @@ vi.mock('./user-dm.js', () => ({
vi.mock('../../config.js', async () => {
const actual = await vi.importActual('../../config.js');
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-channel-approval' };
return {
...actual,
DATA_DIR: '/tmp/nanoclaw-test-channel-approval',
GROUPS_DIR: '/tmp/nanoclaw-test-channel-approval/groups',
};
});
const TEST_DIR = '/tmp/nanoclaw-test-channel-approval';
@@ -176,6 +180,23 @@ describe('unknown-channel registration flow', () => {
expect(count).toBe(1);
});
it('announces the hold through the shared approval-requested observer', async () => {
const { registerApprovalRequestedHandler } = await import('../approvals/primitive.js');
const { routeInbound } = await import('../../router.js');
const events: Array<{ approval: { approval_id: string; action: string }; deliveredTo: string }> = [];
registerApprovalRequestedHandler((event) => {
if (event.approval.action === 'channel_registration') events.push(event);
});
await routeInbound(groupMention('chat-observed'));
await new Promise((r) => setTimeout(r, 10));
expect(events).toHaveLength(1);
expect(events[0].deliveredTo).toBe('telegram:owner');
expect(events[0].approval.approval_id).toMatch(/^mg-/); // the hold view is keyed by the messaging group
});
it('dedups a second mention while the card is pending', async () => {
const { routeInbound } = await import('../../router.js');
await routeInbound(groupMention('chat-busy'));
@@ -358,7 +379,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 approver rule)', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
@@ -395,39 +416,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 = ?')
@@ -438,6 +448,108 @@ describe('unknown-channel registration flow', () => {
.c;
expect(stillPending).toBe(1);
});
it('create new agent: the free-text name reply creates the group and wires the channel', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
await routeInbound(groupMention('chat-create-new'));
await new Promise((r) => setTimeout(r, 10));
const pending = getDb().prepare('SELECT messaging_group_id FROM pending_channel_approvals').get() as {
messaging_group_id: string;
};
// Owner clicks "Connect new agent" → name prompt lands in their DM.
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending.messaging_group_id,
value: 'new_agent',
userId: 'owner',
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
});
if (claimed) break;
}
// Owner replies with the agent name in the same DM — the guarded
// interceptor allows (still an eligible approver) and creates.
await routeInbound({
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
message: {
id: 'name-reply-1',
kind: 'chat' as const,
content: JSON.stringify({ senderId: 'owner', senderName: 'Owner', text: 'Newbie' }),
timestamp: now(),
},
});
const created = getDb().prepare("SELECT id FROM agent_groups WHERE name = 'Newbie'").get() as
| { id: string }
| undefined;
expect(created).toBeDefined();
const mgaCount = (
getDb()
.prepare('SELECT COUNT(*) AS c FROM messaging_group_agents WHERE messaging_group_id = ? AND agent_group_id = ?')
.get(pending.messaging_group_id, created!.id) as { c: number }
).c;
expect(mgaCount).toBe(1);
const stillPending = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_channel_approvals').get() as { c: number })
.c;
expect(stillPending).toBe(0);
});
it('D4 interceptor half: a name reply after the registration vanished is consumed without creating anything', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
await routeInbound(groupMention('chat-vanished'));
await new Promise((r) => setTimeout(r, 10));
const pending = getDb().prepare('SELECT messaging_group_id FROM pending_channel_approvals').get() as {
messaging_group_id: string;
};
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending.messaging_group_id,
value: 'new_agent',
userId: 'owner',
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
});
if (claimed) break;
}
// The registration disappears between the click and the reply (rejected
// from another card, group delete cascade, …) — the guard's baseline no
// longer finds a pending registration, so the reply must not create.
getDb()
.prepare('DELETE FROM pending_channel_approvals WHERE messaging_group_id = ?')
.run(pending.messaging_group_id);
const agentGroupsBefore = (getDb().prepare('SELECT COUNT(*) AS c FROM agent_groups').get() as { c: number }).c;
await routeInbound({
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
message: {
id: 'name-reply-2',
kind: 'chat' as const,
content: JSON.stringify({ senderId: 'owner', senderName: 'Owner', text: 'Ghost' }),
timestamp: now(),
},
});
const agentGroupsAfter = (getDb().prepare('SELECT COUNT(*) AS c FROM agent_groups').get() as { c: number }).c;
expect(agentGroupsAfter).toBe(agentGroupsBefore);
const mgaCount = (getDb().prepare('SELECT COUNT(*) AS c FROM messaging_group_agents').get() as { c: number }).c;
expect(mgaCount).toBe(0);
});
});
describe('no-owner / no-agent failure modes', () => {
+51 -13
View File
@@ -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 { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
import { createPendingChannelApproval, hasInFlightChannelApproval } from './db/pending-channel-approvals.js';
import type { AgentGroup, PendingApproval } from '../../types.js';
import { notifyApprovalRequested, pickApprovalDelivery, pickApprover } from '../approvals/primitive.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,15 +210,19 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
const question = buildQuestionText(isGroup, senderName, channelName, originChannelType);
const options = normalizeOptions(buildApprovalOptions(agentGroups, delivery.userId));
createPendingChannelApproval({
// agent_group_id is NOT NULL bookkeeping (the schema predates the global
// approver chain); it no longer drives approver resolution or click-auth.
const row: PendingChannelApproval = {
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(),
title,
options_json: JSON.stringify(options),
});
};
createPendingChannelApproval(row);
await notifyApprovalRequested({ approval: channelHoldView(row), session: null, deliveredTo: delivery.userId });
const adapter = getDeliveryAdapter();
if (!adapter) {
@@ -271,6 +277,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,
approver_rule: '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);
}
+64
View File
@@ -0,0 +1,64 @@
/**
* Permissions guard adapter catalog entries for the two human-admission
* seams, composed at the module edge (imported by ./index.ts).
*
* channels.register the terminal privileged operations of channel
* registration (create wiring, create agent group, add member) are reachable
* through two doors: the approval-card click (response handler) and the
* free-text name reply (interceptor). Both consult this baseline with the
* clicking/replying human as the actor: the approver rule is the global chain
* owner / global admin plus the specific approver the card was delivered
* to (the D4 fix; previously the anchor was whichever agent group sorted
* first, and the free-text door had no check at all).
*
* senders.admit the unknown_sender_policy decision, verbatim: 'public'
* allows (normally short-circuited before the gate), 'request_approval'
* holds for the wired agent group's admin chain, 'strict' (and anything
* unknown, fail-closed) denies.
*/
import { ALLOW, DENY, HOLD, registerGuardedAction } from '../../guard/index.js';
import { mayResolve } from '../approvals/approver-rule.js';
import { getPendingChannelApproval } from './db/pending-channel-approvals.js';
import { SENDER_ADMIT_ACTION } from './sender-approval.js';
registerGuardedAction({
action: 'channels.register',
baseline: (input) => {
if (input.actor.kind !== 'human' || !input.actor.userId) {
return DENY('channel registration is resolved by a human approver');
}
const messagingGroupId = typeof input.payload.questionId === 'string' ? input.payload.questionId : '';
const row = getPendingChannelApproval(messagingGroupId);
if (!row) {
return DENY(`no pending channel registration for ${messagingGroupId}`);
}
return mayResolve(
{ kind: 'admins-of-scope', agentGroupId: null, deliveredTo: row.approver_user_id },
'group',
input.actor.userId,
)
? ALLOW('eligible channel-registration approver')
: DENY('not an eligible channel-registration approver (owner / global admin)');
},
});
registerGuardedAction({
action: 'senders.admit',
approvalAction: SENDER_ADMIT_ACTION,
baseline: (input) => {
const policy = input.payload.policy;
if (policy === 'public') return ALLOW('public messaging group');
if (policy === 'request_approval') {
return HOLD(
{
kind: 'admins-of-scope',
agentGroupId: (input.payload.agentGroupId as string | undefined) ?? null,
deliveredTo: null,
},
'group',
'unknown sender requires admission approval',
);
}
return DENY(`unknown sender (policy ${String(policy)})`);
},
});
+118 -115
View File
@@ -15,6 +15,7 @@
* Without this module: sender resolution is a no-op (userId=null); the
* access gate is not registered and core defaults to allow-all.
*/
import './guard.js';
import { recordDroppedMessage } from '../../db/dropped-messages.js';
import { getAgentGroup, getAllAgentGroups } from '../../db/agent-groups.js';
import { createMessagingGroupAgent, setMessagingGroupDeniedAt } from '../../db/messaging-groups.js';
@@ -30,11 +31,14 @@ import {
import type { InboundEvent } from '../../channels/adapter.js';
import { registerResponseHandler, type ResponsePayload } from '../../response-registry.js';
import { getDeliveryAdapter } from '../../delivery.js';
import { guard } from '../../guard/index.js';
import { log } from '../../log.js';
import type { MessagingGroup, MessagingGroupAgent } from '../../types.js';
import { notifyApprovalResolved, registerApprovalHandler } from '../approvals/primitive.js';
import { canAccessAgentGroup } from './access.js';
import {
buildAgentSelectionOptions,
channelHoldView,
CHOOSE_EXISTING_VALUE,
CONNECT_PREFIX,
createNewAgentGroup,
@@ -48,10 +52,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 ──
@@ -129,43 +132,50 @@ function handleUnknownSender(
agent_group_id: agentGroupId,
};
if (mg.unknown_sender_policy === 'strict') {
log.info('MESSAGE DROPPED — unknown sender (strict policy)', {
// The admission decision is the guard's senders.admit baseline (./guard.ts)
// — unknown_sender_policy verbatim: strict → deny, request_approval → hold,
// public → allow (short-circuited before the gate). Drop-recording and the
// hold creation stay here.
const decision = guard({
action: 'senders.admit',
actor: userId ? { kind: 'human', userId } : { kind: 'system' },
payload: {
messagingGroupId: mg.id,
agentGroupId,
senderIdentity: userId,
policy: mg.unknown_sender_policy,
},
});
if (decision.effect === 'allow') return; // 'public' — handled before the gate; fall through silently.
log.info(
decision.effect === 'hold'
? 'MESSAGE DROPPED — unknown sender (approval requested)'
: 'MESSAGE DROPPED — unknown sender (strict policy)',
{
messagingGroupId: mg.id,
agentGroupId,
userId,
accessReason,
});
recordDroppedMessage(dropRecord);
return;
}
},
);
recordDroppedMessage(dropRecord);
if (mg.unknown_sender_policy === 'request_approval') {
log.info('MESSAGE DROPPED — unknown sender (approval requested)', {
// Fire-and-forget; pick-approver + delivery + row-insert are all async.
// If it fails it logs internally — the user's message still stays dropped
// either way. Requires a resolved userId (senderResolver populates users
// row before the gate fires); if we got here without one, there's nothing
// to identify for approval and we just drop silently.
if (decision.effect === 'hold' && userId) {
requestSenderApproval({
messagingGroupId: mg.id,
agentGroupId,
userId,
accessReason,
});
recordDroppedMessage(dropRecord);
// Fire-and-forget; pick-approver + delivery + row-insert are all async.
// If it fails it logs internally — the user's message still stays dropped
// either way. Requires a resolved userId (senderResolver populates users
// row before the gate fires); if we got here without one, there's nothing
// to identify for approval and we just stay in the "silent strict" branch.
if (userId) {
requestSenderApproval({
messagingGroupId: mg.id,
agentGroupId,
senderIdentity: userId,
senderName,
event,
}).catch((err) => log.error('Sender-approval flow threw', { err }));
}
return;
senderIdentity: userId,
senderName,
event,
}).catch((err) => log.error('Sender-approval flow threw', { err }));
}
// 'public' should have been handled before the gate; fall through silently.
}
setSenderResolver(extractAndUpsertUser);
@@ -209,83 +219,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,27 +275,32 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
const row = getPendingChannelApproval(payload.questionId);
if (!row) return false;
// Click authorization (owner / global admin / delivered-to approver) is the
// guard's channels.register baseline, consulted by the response registry's
// wrapper before this handler runs.
//
// 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('Channel registration click rejected — unauthorized clicker', {
messagingGroupId: row.messaging_group_id,
clickerId,
expectedApprover: row.approver_user_id,
});
return true;
}
if (!clickerId) return true; // unreachable behind the guard wrapper; fail closed
const approverId = clickerId;
// ── 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);
@@ -515,13 +490,19 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
return true;
}
registerResponseHandler(handleChannelApprovalResponse);
registerResponseHandler(handleChannelApprovalResponse, {
action: 'channels.register',
claims: (payload) => getPendingChannelApproval(payload.questionId) !== undefined,
});
// ── Free-text name interceptor ──
// Captures the next DM from an approver who clicked "Create new agent",
// creates the agent immediately, wires the channel, and replays.
// creates the agent immediately, wires the channel, and replays. The router
// wraps it with the guard (D4's interceptor half): the free-texter must still
// be an eligible channel-registration approver at reply time — a role revoked
// between the click and the reply now denies, and the arming is disarmed.
registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
const captureAgentNameReply = async (event: InboundEvent): Promise<boolean> => {
const userId = extractAndUpsertUser(event);
if (!userId) return false;
@@ -603,6 +584,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);
@@ -629,4 +616,20 @@ registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
}
}
return true;
};
registerMessageInterceptor(captureAgentNameReply, {
action: 'channels.register',
claims: (event) => {
const userId = extractAndUpsertUser(event);
if (!userId) return null;
const pending = awaitingNameInput.get(userId);
if (!pending) return null;
if (event.channelType !== pending.dmChannelType || event.platformId !== pending.dmPlatformId) return null;
return { actor: { kind: 'human', userId }, payload: { questionId: pending.channelMgId } };
},
onDeny: (event) => {
const userId = extractAndUpsertUser(event);
if (userId) awaitingNameInput.delete(userId);
},
});
+67 -36
View File
@@ -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;
approver_rule: string;
approver_user_id: string;
dedup_key: string;
}>;
expect(rows).toHaveLength(1);
// Hold-record contract: sessionless, anchored to the agent group,
// named-or-admin approver rule with the delivered approver recorded.
expect(rows[0].session_id).toBeNull();
expect(rows[0].agent_group_id).toBe('ag-1');
expect(rows[0].approver_rule).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');
+33 -121
View File
@@ -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 rule: 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';
+21 -18
View File
@@ -1,11 +1,12 @@
/**
* Approval handlers for self-modification actions.
* Guarded handler bodies for self-modification actions.
*
* The approvals module calls these when an admin clicks Approve on a
* pending_approvals row whose action matches. Each handler mutates the
* container config in the DB, rebuilds/kills the container as needed,
* and writes an on_wake message so the fresh container picks up where
* the old one left off.
* The delivery registry's guard wrapper runs these only on `allow` which,
* for self-mod, means an approved replay carrying a valid grant (the
* baseline holds unconditionally from the container path; see ./guard.ts).
* Each body mutates the container config in the DB, rebuilds/kills the
* container as needed, and writes an on_wake message so the fresh container
* picks up where the old one left off.
*
* install_packages: update DB + rebuild image + kill container + on_wake.
* add_mcp_server: update DB + kill container + on_wake.
@@ -17,18 +18,19 @@ import { getSession } from '../../db/sessions.js';
import type { McpServerConfig } from '../../container-config.js';
import { log } from '../../log.js';
import { writeSessionMessage } from '../../session-manager.js';
import type { ApprovalHandler } from '../approvals/index.js';
import type { Session } from '../../types.js';
import { notifyAgent } from '../approvals/index.js';
export const applyInstallPackages: ApprovalHandler = async ({ session, payload, userId, notify }) => {
export async function applyInstallPackages(payload: Record<string, unknown>, session: Session): Promise<void> {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) {
notify('install_packages approved but agent group missing.');
notifyAgent(session, 'install_packages approved but agent group missing.');
return;
}
const configRow = getContainerConfig(agentGroup.id);
if (!configRow) {
notify('install_packages approved but container config missing.');
notifyAgent(session, 'install_packages approved but container config missing.');
return;
}
@@ -52,7 +54,7 @@ export const applyInstallPackages: ApprovalHandler = async ({ session, payload,
...((payload.apt as string[] | undefined) || []),
...((payload.npm as string[] | undefined) || []),
].join(', ');
log.info('Package install approved', { agentGroupId: session.agent_group_id, userId });
log.info('Package install approved', { agentGroupId: session.agent_group_id });
try {
await buildAgentGroupImage(session.agent_group_id);
writeSessionMessage(session.agent_group_id, session.id, {
@@ -75,23 +77,24 @@ export const applyInstallPackages: ApprovalHandler = async ({ session, payload,
});
log.info('Container rebuild completed (bundled with install)', { agentGroupId: session.agent_group_id });
} catch (e) {
notify(
notifyAgent(
session,
`Packages added to config (${pkgs}) but rebuild failed: ${e instanceof Error ? e.message : String(e)}. Tell the user — an admin will need to retry the install_packages request or inspect the build logs.`,
);
log.error('Bundled rebuild failed after install approval', { agentGroupId: session.agent_group_id, err: e });
}
};
}
export const applyAddMcpServer: ApprovalHandler = async ({ session, payload, userId, notify }) => {
export async function applyAddMcpServer(payload: Record<string, unknown>, session: Session): Promise<void> {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) {
notify('add_mcp_server approved but agent group missing.');
notifyAgent(session, 'add_mcp_server approved but agent group missing.');
return;
}
const configRow = getContainerConfig(agentGroup.id);
if (!configRow) {
notify('add_mcp_server approved but container config missing.');
notifyAgent(session, 'add_mcp_server approved but container config missing.');
return;
}
@@ -122,5 +125,5 @@ export const applyAddMcpServer: ApprovalHandler = async ({ session, payload, use
const s = getSession(session.id);
if (s) wakeContainer(s);
});
log.info('MCP server add approved', { agentGroupId: session.agent_group_id, userId });
};
log.info('MCP server add approved', { agentGroupId: session.agent_group_id });
}
+36
View File
@@ -0,0 +1,36 @@
/**
* Self-mod guard adapter the module's catalog entries, composed at the
* module edge (imported by ./index.ts).
*
* The structural baseline is today's behavior verbatim: from the container
* path, self-modification is held unconditionally for the agent group's
* admin chain. (The equivalent host-side mutations `ncl groups config
* add-package` etc. — are separate catalog actions derived from the command
* registry.)
*/
import { DENY, HOLD, registerGuardedAction, type GuardInput } from '../../guard/index.js';
function selfModBaseline(label: string) {
return (input: GuardInput) => {
if (input.actor.kind !== 'agent') {
return DENY(`${label} is a container-originated action.`);
}
return HOLD(
{ kind: 'admins-of-scope', agentGroupId: input.actor.agentGroupId, deliveredTo: null },
'group',
`${label} always requires admin approval from the container path`,
);
};
}
registerGuardedAction({
action: 'self_mod.install_packages',
approvalAction: 'install_packages',
baseline: selfModBaseline('install_packages'),
});
registerGuardedAction({
action: 'self_mod.add_mcp_server',
approvalAction: 'add_mcp_server',
baseline: selfModBaseline('add_mcp_server'),
});
+36 -14
View File
@@ -2,29 +2,51 @@
* Self-modification module admin-approved container mutations.
*
* Optional tier. Depends on the approvals default module for the request/
* handler plumbing. On install the module registers:
* - Two delivery actions (install_packages, add_mcp_server) that validate
* input and queue an approval via requestApproval().
* - Two matching approval handlers that run on approve and perform the
* complete follow-up:
* install_packages update container.json, rebuild image, kill
* handler plumbing and on the guard for the decision. On install the module
* registers:
* - Its guard-catalog entries (./guard.ts): unconditional hold from the
* container path.
* - Two guard-wrapped delivery actions (install_packages, add_mcp_server):
* validation runs as the wrapper's precheck, the hold builders card the
* admin, and the handler bodies (./apply.ts) run only on allow i.e. on
* an approved replay:
* install_packages update container_configs, rebuild image, kill
* container (next wake respawns on the new image), schedule a
* verify-and-report follow-up prompt.
* add_mcp_server update container.json, kill container. No image
* add_mcp_server update container_configs, kill container. No image
* rebuild bun runs TS directly, so the new MCP server is wired
* by the next container start.
* - Two approval handlers that re-enter the wrapped actions with the
* approval row as the grant (one replay semantics the guard re-checks
* the structural baseline live).
*
* Without this module: the MCP tools in the container still write outbound
* system messages with these actions, but delivery logs "Unknown system
* action" and drops them. Admin never sees a card; nothing changes.
*/
import { registerDeliveryAction } from '../../delivery.js';
import { registerApprovalHandler } from '../approvals/index.js';
import './guard.js';
import { reenterGuardedDeliveryAction, registerDeliveryAction } from '../../delivery.js';
import { notifyAgent, registerApprovalHandler } from '../approvals/index.js';
import { applyAddMcpServer, applyInstallPackages } from './apply.js';
import { handleAddMcpServer, handleInstallPackages } from './request.js';
import {
requestAddMcpServerHold,
requestInstallPackagesHold,
validateAddMcpServer,
validateInstallPackages,
} from './request.js';
registerDeliveryAction('install_packages', handleInstallPackages);
registerDeliveryAction('add_mcp_server', handleAddMcpServer);
registerDeliveryAction('install_packages', applyInstallPackages, {
guardAction: 'self_mod.install_packages',
precheck: validateInstallPackages,
requestHold: requestInstallPackagesHold,
onDeny: (_content, session, reason) => notifyAgent(session, `install_packages denied: ${reason}`),
});
registerDeliveryAction('add_mcp_server', applyAddMcpServer, {
guardAction: 'self_mod.add_mcp_server',
precheck: validateAddMcpServer,
requestHold: requestAddMcpServerHold,
onDeny: (_content, session, reason) => notifyAgent(session, `add_mcp_server denied: ${reason}`),
});
registerApprovalHandler('install_packages', applyInstallPackages);
registerApprovalHandler('add_mcp_server', applyAddMcpServer);
registerApprovalHandler('install_packages', reenterGuardedDeliveryAction('install_packages'));
registerApprovalHandler('add_mcp_server', reenterGuardedDeliveryAction('add_mcp_server'));
+32 -16
View File
@@ -1,12 +1,12 @@
/**
* Delivery-action handlers for agent-initiated self-modification requests.
* Validation + hold-request builders for agent-initiated self-modification.
*
* Two actions the container can write into messages_out (via the self-mod
* MCP tools): install_packages, add_mcp_server. Each one validates input
* and queues an approval request. The admin's approval triggers the
* matching approval handler in ./apply.ts, which also performs the
* required follow-up (rebuild+restart for install_packages, restart-only
* for add_mcp_server).
* MCP tools): install_packages, add_mcp_server. The delivery registry wraps
* each one with the guard (see ./guard.ts unconditional hold from the
* container path): validation here runs as the wrapper's precheck, and the
* hold builders create the approval card when the guard holds. On approve,
* the continuation re-enters the wrapped action and ./apply.ts runs.
*
* Host-side sanitization for install_packages is defense-in-depth the MCP
* tool validates first. Both layers matter: the DB row carries the payload
@@ -17,40 +17,48 @@ import { log } from '../../log.js';
import type { Session } from '../../types.js';
import { notifyAgent, requestApproval } from '../approvals/index.js';
export async function handleInstallPackages(content: Record<string, unknown>, session: Session): Promise<void> {
export function validateInstallPackages(content: Record<string, unknown>, session: Session): boolean {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) {
notifyAgent(session, 'install_packages failed: agent group not found.');
return;
return false;
}
const apt = (content.apt as string[]) || [];
const npm = (content.npm as string[]) || [];
const reason = (content.reason as string) || '';
const APT_RE = /^[a-z0-9][a-z0-9._+-]*$/;
const NPM_RE = /^(@[a-z0-9][a-z0-9._-]*\/)?[a-z0-9][a-z0-9._-]*$/;
const MAX_PACKAGES = 20;
if (apt.length + npm.length === 0) {
notifyAgent(session, 'install_packages failed: at least one apt or npm package is required.');
return;
return false;
}
if (apt.length + npm.length > MAX_PACKAGES) {
notifyAgent(session, `install_packages failed: max ${MAX_PACKAGES} packages per request.`);
return;
return false;
}
const invalidApt = apt.find((p) => !APT_RE.test(p));
if (invalidApt) {
notifyAgent(session, `install_packages failed: invalid apt package name "${invalidApt}".`);
log.warn('install_packages: invalid apt package rejected', { pkg: invalidApt });
return;
return false;
}
const invalidNpm = npm.find((p) => !NPM_RE.test(p));
if (invalidNpm) {
notifyAgent(session, `install_packages failed: invalid npm package name "${invalidNpm}".`);
log.warn('install_packages: invalid npm package rejected', { pkg: invalidNpm });
return;
return false;
}
return true;
}
export async function requestInstallPackagesHold(content: Record<string, unknown>, session: Session): Promise<void> {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) return;
const apt = (content.apt as string[]) || [];
const npm = (content.npm as string[]) || [];
const reason = (content.reason as string) || '';
const packageList = [...apt.map((p) => `apt: ${p}`), ...npm.map((p) => `npm: ${p}`)].join(', ');
await requestApproval({
@@ -63,18 +71,26 @@ export async function handleInstallPackages(content: Record<string, unknown>, se
});
}
export async function handleAddMcpServer(content: Record<string, unknown>, session: Session): Promise<void> {
export function validateAddMcpServer(content: Record<string, unknown>, session: Session): boolean {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) {
notifyAgent(session, 'add_mcp_server failed: agent group not found.');
return;
return false;
}
const serverName = content.name as string;
const command = content.command as string;
if (!serverName || !command) {
notifyAgent(session, 'add_mcp_server failed: name and command are required.');
return;
return false;
}
return true;
}
export async function requestAddMcpServerHold(content: Record<string, unknown>, session: Session): Promise<void> {
const agentGroup = getAgentGroup(session.agent_group_id);
if (!agentGroup) return;
const serverName = content.name as string;
const command = content.command as string;
await requestApproval({
session,
agentName: agentGroup.name,
+49 -5
View File
@@ -7,10 +7,19 @@
* which triggers module registrations that would otherwise happen before
* index.ts's own const initializers have run.
*
* Keep this file dependency-free (log.js is fine, but nothing from
* modules/* or index.ts itself). Any file imported here must not in turn
* import from src/index.ts, or the cycle returns.
* Keep this file dependency-free (log.js and the guard leaf are fine, but
* nothing from modules/* or index.ts itself). Any file imported here must
* not in turn import from src/index.ts, or the cycle returns.
*
* A handler whose click performs a privileged operation registers with a
* guard spec: the registry wraps it so the guard's decision stands between
* the click and the handler, and the wrapped path is the only path. `claims`
* is the handler's own claim test (does this questionId belong to me?) so an
* unauthorized click is claimed-and-dropped without stealing other handlers'
* responses.
*/
import { guard, type GuardActor } from './guard/index.js';
import { log } from './log.js';
export interface ResponsePayload {
questionId: string;
@@ -23,10 +32,45 @@ export interface ResponsePayload {
export type ResponseHandler = (payload: ResponsePayload) => Promise<boolean>;
export interface ResponseGuardSpec {
/** Dotted guard-catalog action consulted before the handler runs. */
action: string;
/** Would this handler claim the response? (Its own row lookup.) */
claims: (payload: ResponsePayload) => boolean;
}
const responseHandlers: ResponseHandler[] = [];
export function registerResponseHandler(handler: ResponseHandler): void {
responseHandlers.push(handler);
function responseActor(payload: ResponsePayload): GuardActor {
if (!payload.userId) return { kind: 'human', userId: '' };
const userId = payload.userId.includes(':') ? payload.userId : `${payload.channelType}:${payload.userId}`;
return { kind: 'human', userId };
}
export function registerResponseHandler(handler: ResponseHandler, guardSpec?: ResponseGuardSpec): void {
if (!guardSpec) {
responseHandlers.push(handler);
return;
}
responseHandlers.push(async (payload) => {
if (!guardSpec.claims(payload)) return false;
const decision = guard({
action: guardSpec.action,
actor: responseActor(payload),
payload: { questionId: payload.questionId, value: payload.value },
});
if (decision.effect !== 'allow') {
// Claim the response so it's not unclaimed-logged, but do nothing.
log.warn('Response click rejected by guard', {
action: guardSpec.action,
questionId: payload.questionId,
userId: payload.userId,
reason: decision.reason,
});
return true;
}
return handler(payload);
});
}
export function getResponseHandlers(): readonly ResponseHandler[] {
+37 -2
View File
@@ -19,6 +19,7 @@
*/
import { getChannelAdapter } from './channels/channel-registry.js';
import { gateCommand } from './command-gate.js';
import { guard, type GuardActor } from './guard/index.js';
import { getAgentGroup } from './db/agent-groups.js';
import { recordDroppedMessage } from './db/dropped-messages.js';
import {
@@ -117,13 +118,47 @@ export function setSenderScopeGate(fn: SenderScopeGateFn): void {
* Used by modules to capture free-text DM replies during multi-step approval
* flows the permissions module (agent naming during channel registration)
* and the approvals module (reject-with-reason capture).
*
* An interceptor whose capture performs a privileged operation (the
* channel-registration name capture creates an agent group + wiring)
* registers with a guard spec: the registry wraps it so the guard's decision
* stands between the free-text reply and the handler the D4 fix's
* interceptor half. `claims` returns the guard consult for events the
* interceptor would act on (null = not mine, pass through); a deny consumes
* the message without acting.
*/
export type MessageInterceptorFn = (event: InboundEvent) => Promise<boolean>;
export interface InterceptorGuardSpec {
/** Dotted guard-catalog action consulted before the interceptor acts. */
action: string;
/** The guard consult for events this interceptor would act on; null = not mine. */
claims: (event: InboundEvent) => { actor: GuardActor; payload: Record<string, unknown> } | null;
/** Domain cleanup when the guard denies (e.g. disarm the capture). */
onDeny?: (event: InboundEvent) => void;
}
const messageInterceptors: MessageInterceptorFn[] = [];
export function registerMessageInterceptor(fn: MessageInterceptorFn): void {
messageInterceptors.push(fn);
export function registerMessageInterceptor(fn: MessageInterceptorFn, guardSpec?: InterceptorGuardSpec): void {
if (!guardSpec) {
messageInterceptors.push(fn);
return;
}
messageInterceptors.push(async (event) => {
const consult = guardSpec.claims(event);
if (!consult) return fn(event);
const decision = guard({ action: guardSpec.action, actor: consult.actor, payload: consult.payload });
if (decision.effect !== 'allow') {
log.warn('Interceptor capture rejected by guard — consuming without acting', {
action: guardSpec.action,
reason: decision.reason,
});
guardSpec.onDeny?.(event);
return true;
}
return fn(event);
});
}
/**
+25 -1
View File
@@ -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/approver-rule.ts (`mayResolve`).
*/
export type ApproverRule =
| { 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 `approver_rule: '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/approver-rule.ts. */
approver_rule: '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) ──