Compare commits

..

13 Commits

Author SHA1 Message Date
gavrielc 4a3e01cab8 Merge branch 'main' into feat/lean-harness-defaults 2026-07-13 16:32:28 +03:00
gavrielc b52c8c6dca Update src/group-init.settings.test.ts 2026-07-13 16:32:00 +03:00
gavrielc 69913f23ec Update CHANGELOG.md 2026-07-13 16:31:48 +03:00
gavrielc 1de7dba2b7 Update src/group-init.ts 2026-07-13 16:31:40 +03:00
gavrielc 37f60c831d Update src/group-init.ts 2026-07-13 16:31:27 +03:00
gavrielc 6bf3b3929e Update src/group-init.settings.test.ts 2026-07-13 16:31:16 +03:00
github-actions[bot] 2d3257c601 chore: bump version to 2.1.47 2026-07-13 11:24:03 +00:00
github-actions[bot] a467359d86 docs: update token count to 240k tokens · 120% of context window 2026-07-13 11:24:00 +00:00
Moshe Krupper 40d26893f8 Guard seam: one decision function for every privileged action (guarded-actions phase 2) (#2986)
* feat: guard seam — decision function, registration wrapping, grant-carrying replay, boot conformance

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 | deny (hub:
engineering/requirements/guarded-actions +
engineering/discovery/guarded-actions-decisions, phase 2).

- Registration-derived catalog: registry.register() derives one entry
  per ncl command (dotted action names stamped by registerResource);
  module-edge guard.ts adapters register the domain baselines
  (agents.create, a2a.send incl. the agent_message_policies hold,
  self_mod.*, senders.admit, channels.register). The baseline is the
  decision — policy-as-data (tighten-only rules) is deferred to
  phase 3.
- All four handler registries wrap at registration: dispatch consults
  the guard; guarded delivery actions (create_agent, install_packages,
  add_mcp_server) store only the precheck → guard → handler wrapper
  (the raw handler is never stored; spec-less re-registration throws);
  response handlers + message interceptors take guard specs (channel
  registration's click + free-text name capture).
- Grant-carrying replay: approved continuations re-enter their entry
  point with the verified approval row as the grant
  (ApprovalHandlerContext.approval). A grant satisfies a hold, never a
  deny — live-row + approvalAction + grantMatches checks; the forgeable
  approved:true boolean is deleted.
- Boot conformance: the registry walk (src/guard-conformance.ts) runs
  in CI and at boot — an unmapped privileged registration stops the
  host with a banner (skill-installed code never runs this repo's CI).

Baselines are main's behavior verbatim (host trusted-caller, cli_scope
allowlist, create_agent scope branch, a2a ACL order, unconditional
self-mod hold, unknown_sender_policy, channel click auth incl. the
anchor-group approver). Sender/channel holds stay on their own tables;
guard holds map onto the existing requestApproval options
(approverUserId).

Deliberate outcome changes inherent to the replay semantics (called out
in CHANGELOG too): (a) a2a approve-then-revoke no longer delivers — the
structural baseline re-runs live on replay; (b) forged, already-consumed,
or mismatched grants refuse instead of executing; (c) the channel-name
free-text reply re-checks approver eligibility at reply time. The
D1/D2/D4 click-auth fixes are deliberately NOT here — they belong to the
approval-contract PR (D2's host fallback at dispatch replay is preserved
verbatim).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor: rename src/guard/catalog.ts → guard-actions.ts

File + internal map (catalog → guardedActions). The concept stays "the
action catalog" in prose (the term the requirements/decisions docs and
the conformance banner use); exported symbols (registerGuardedAction,
GuardedActionSpec, …) unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(guard): consults carry the defined action value — delete the registry walk

The guard's weak point was the wiring: catalog entries registered by
side-effect import, consult sites naming them by string, a map miss
resolving to ALLOW, and a boot walk that could only see 2 of the 4
registries (response handlers and interceptors erased their specs into
closures). Dropping one unreferenced import silently disarmed
channel-registration click-auth.

Make the broken wiring unconstructible instead of detected:

- defineGuardedAction returns a branded GuardedAction value; guard(action,
  input) takes the value, not a name. A dropped module-edge import or a
  typo'd action is now a compile error; there is no lookup and no
  fail-open branch. A forged value (outside TS) is denied at runtime.
- Every registry (delivery actions, response handlers, interceptors)
  requires a guard spec or an explicit unguarded(<reason>) declaration at
  the registration site — unguarded-by-omission is not representable, and
  the justification travels with the registration (grep "unguarded(" for
  the complete inventory). CLI commands derive their guard inside
  register(), so a command cannot exist without one.
- guardConformanceViolations() and EXEMPT_DELIVERY_ACTIONS are gone. The
  boot check shrinks to the one cross-registry invariant the compiler
  can't see: every holding action has a registered approve continuation
  (grantContinuationGaps), same fail-closed refuse-to-start posture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(guard): one runtime discriminator for Unguarded — isUnguarded()

The unguarded brand becomes a real (module-private) symbol and
isUnguarded() its exported type guard, replacing three hand-rolled
`'reason' in guardDecl` checks (delivery, router, response-registry)
plus the one inside isUnguardedEntry. Only unguarded() can mint the
brand now, so a look-alike { reason } object — or a guard spec that
someday grows a `reason` field — can't pass as an unguarded
declaration at runtime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(guard): unwrap the broadcast registries — response handlers and interceptors consult inline

registerResponseHandler and registerMessageInterceptor return to main's
single-argument shape: the guard-spec/unguarded declaration machinery
(claims, onDeny, the registry wrappers) is deleted. The declaration
requirement stays where registration is keyed — every ncl command derives
its guard inside register(), and registerDeliveryAction demands a spec or
unguarded(<reason>) — while broadcast hooks gate inline at the point of
privilege, like the a2a route and the unknown-sender gate:

- handleChannelApprovalResponse consults guard(channelsRegister) inline
  where main had the click-auth if — same decision, same timing.
- The free-text name capture returns to main verbatim: the click arms it,
  the reply is not re-authorized (the body's own pending-row re-fetch
  still refuses a vanished registration). Behavior delta (c) is withdrawn
  — the deliberate deltas are back to (a) approve-then-revoke and
  (b) grant refusals.

Also gone with the wrappers: the claims/body predicate duplication (the
same early-exits existed twice and could drift apart) and the repeated
extractAndUpsertUser upsert per captured event.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(guard): drop boot conformance, rename the spec vocabulary, extract runGuarded

Three trims to the seam, no behavior change (713/713 green):

- Boot-time conformance check deleted (src/guard-conformance.ts + its
  main() call). The invariant — every holding action has a registered
  approve continuation — is already covered in-tree by the conformance
  test, and at runtime response-handler.ts handles a missing continuation
  loudly (warn + "approved, but no handler is installed" notify + row
  cleanup). Refusing to boot bought no safety over that and could brick
  the host on a mis-installed skill.

- GuardedActionSpec vocabulary renamed to read as what each field does:
  baseline → decide (the decision function — allow | hold | deny),
  approvalAction → grantActionName (the pending_approvals.action string a
  grant's row must carry; "Name" because at consult sites `action` means
  the branded GuardedAction value), grantMatches → grantCoversRequest.
  Comment prose follows ("structural baseline" → the checks / the
  decision).

- runGuarded + DeliveryGuardSpec extracted to src/delivery-guard.ts.
  delivery.ts is a high-traffic file for forks: the registry itself
  (tagged entries, spec-or-unguarded registration overloads, disarm-throw,
  one-door getDeliveryAction, grant-carrying reenter) stays there, evolved
  in place — only the consult pipeline (precheck → guard → deny/hold/allow)
  and the spec type move out; runGuarded takes spec + handler explicitly so
  the new file has no import back into delivery.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: drop internal design-process references from the guard change

The public tree shouldn't point at internal artifacts: phase numbering
("guarded-actions phase 2/3") and the team-hub decisions-doc pointer are
gone from the CHANGELOG bullet, the CLAUDE.md guard row, and the guard
file headers. The facts stay ("policy-as-data is deliberately deferred —
a generalized rules table can arrive later"); only the internal roadmap
vocabulary goes. Scope: lines this branch introduced only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(guard): the name reply is not re-authorized — stop claiming it is

The channelsRegister docstring and the CLAUDE.md guard row still described
the withdrawn reply-time re-check (behavior delta (c), removed in fe235c7):
the docstring claimed the decision is consulted by the name-capture
interceptor "so a privilege revoked mid-flow is re-checked at each step",
and the CLAUDE.md row said message interceptors consult the guard inline.
Neither is true — channels.register is consulted only by the card-click
handler, and the free-text reply deliberately authorizes off the click
(main's behavior). Both docs now state that explicitly, including the
consequence: a privilege revoked between click and reply still completes
the flow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(guard): an approved-then-refused a2a replay is a policy outcome, not a crash (GS-002)

Runtime testing (PR #2986 validation, finding GS-002) showed the
approve-after-revoke refusal — the PR's own deliberate behavior delta —
surfacing as "ERROR Approval handler threw" with a stack trace, and the
requester being told the apply "failed". Enforcement was correct; the
telemetry dressed an expected guard refusal as a runtime failure.

The a2a route's deny now throws a typed GuardDenyError (guard leaf), and
applyA2aMessageGate catches exactly that: the requester is notified
"Message approved, but not delivered — no longer authorized: <reason>"
and the host logs a warning. Anything else still propagates to the
response handler's crash path. Covers all three replay refusals the same
way: destination revoked while pending, mismatched grant, already-consumed
grant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: shrink the guard CHANGELOG bullets and CLAUDE.md rows to house length

The guard-seam CHANGELOG bullet was a design document; it now reads like
the neighboring entries — what changed, the two behavior deltas, done.
Same for the src/guard/ and src/delivery-guard.ts Key Files rows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: gavrielc <gabicohen22@yahoo.com>
2026-07-13 14:23:45 +03:00
gavrielc 9390037761 Merge pull request #3021 from nanocoai/whatsapp-shared-warning
Warn before connecting a shared WhatsApp number
2026-07-13 12:51:23 +03:00
Gabi Simons 4f5440d2ab feat: lean harness defaults for new agent groups
New groups' settings.json no longer enables Claude Code's experimental
agent-teams feature and sets disableWorkflows: both overlap NanoClaw's
own systems (a2a messaging; host-side orchestration), and together they
are ~20% of every turn's context (live-measured: ~38,900 -> ~30,300
tokens on a fresh session). DesignSync and ReportFindings — desktop/UI
tools that cannot function headless — join the fixed disallow list.

Existing groups are untouched: their settings.json already exists and
is only ever patched additively, never regenerated. Re-enabling either
feature for one group is a one-file edit to that group's settings.json
plus a group restart — and it sticks, because nothing reconciles the
file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 09:52:13 +03:00
Koshkoshinsk ef5d09680a fix: render WhatsApp warning title cleanly 2026-07-12 13:22:28 +03:00
Koshkoshinsk 33b33b84b9 fix: warn before shared WhatsApp setup 2026-07-12 13:05:36 +03:00
59 changed files with 1174 additions and 3855 deletions
-58
View File
@@ -1,58 +0,0 @@
# Remove /add-audit
Reverses every change the skill made. Safe to re-run even if some pieces are
already gone. Run from the NanoClaw project root.
## 1. Delete the copied files
```bash
rm -rf src/audit
rm -f src/cli/dispatch.audit.ts src/cli/dispatch.audit.test.ts
rm -f src/cli/resources/audit.ts
rm -f src/modules/approvals/approvals.audit.ts src/modules/approvals/approvals.audit.test.ts
rm -f src/audit-wiring.test.ts
```
## 2. Revert the dispatch composition
In `src/cli/dispatch.ts`, delete (not comment out) the three edits the skill
made:
1. The import line: `import { withAudit } from './dispatch.audit.js';`
2. The composition block (comment + `export const dispatch = withAudit(dispatchInner);`)
3. Rename the dispatcher back — change `async function dispatchInner(` to
`export async function dispatch(`
## 3. Unregister the resource
Delete the `import './audit.js';` line from `src/cli/resources/index.ts`.
## 4. Unwire the approval observers
Delete the `import './approvals.audit.js';` line (and its comment) from
`src/modules/approvals/index.ts`.
## 5. Remove the settings
Delete the `AUDIT_ENABLED` and `AUDIT_RETENTION_DAYS` lines from `.env`.
## 6. Rebuild and restart
```bash
pnpm run build
source setup/lib/install-slug.sh
launchctl kickstart -k gui/$(id -u)/$(launchd_label) # macOS
# systemctl --user restart $(systemd_unit) # Linux
```
## Day-files
`data/audit/*.ndjson` are the operator's records and are deliberately left in
place. To purge them too:
```bash
rm -rf data/audit
```
No dependency was added, nothing under `container/` was touched, and no DB
schema changed — there is nothing else to undo.
-265
View File
@@ -1,265 +0,0 @@
---
name: add-audit
description: Add an opt-in local audit log for the ncl command surface and every host-routed approval — each command's dispatch outcome on both transports (host socket and container, scope denials included), plus each approval's request and decision (approved/rejected, across CLI, self-mod, a2a, sender admission, OneCLI, channel registration), written as SIEM-shaped append-only NDJSON day-files under data/audit/. Read back with `ncl audit list`; plug exporters in via registerAuditHook. Off until AUDIT_ENABLED=true.
---
# /add-audit — Local Audit Log (ncl surface + approvals)
Records one canonical audit event for two surfaces:
1. **Every `ncl` command** — human over the host socket or agent over the
container transport, denials included. Both transports converge on the
exported `dispatch`, so one composition covers the whole surface: no second
door.
2. **Every host-routed approval** — its `pending` request and its
`approvals.decide` decision (approved / rejected), whichever stack gated it:
CLI commands, self-mod (`install_packages` / `add_mcp_server`), a2a
create-agent and message-gate, unknown-sender admission, OneCLI credential
holds, and channel registration. Two lifecycle observers cover them all with
zero touch points inside the approval flows.
```
ncl (host socket) ──┐
├─→ dispatch = withAudit(dispatchInner) ─→┐
container cli_request ┘ │ │
approved replay (grant) ───────┘ ├─→ one NDJSON line
│ data/audit/<UTC-day>.ndjson
every hold ─→ requested observer ─→ pending ─────────────────┤
every resolution ─→ resolved observer ─→ approvals.decide ───┘
```
What one event carries: `actor` (`host:<os-user>`, the agent group, a
`<channel>:<handle>` admin for a decision, or `system` for an expiry/sweep),
`origin` (transport, session, channel), dotted `action` from the guard catalog
(e.g. `groups.config.add-mcp-server`, `agents.create`, `approvals.decide`),
touched/attempted `resources` (a gated chain names its `approval` and, on the
`pending` event, the picked approver as a `user`), `outcome` (`success ·
failure · denied · pending · approved · rejected`), `correlation_id` (the
approval id on gated chains — the `pending` request, the `approvals.decide`
decision, and a CLI replay's terminal event all share it, so `--correlation
<id>` returns the whole chain), and `details` (governance-safe names only —
never raw argument values).
Two adapters, one leaf: the CLI adapter (`cli/dispatch.audit.ts`) owns command
events and the CLI hold's `pending`; the approvals adapter
(`modules/approvals/approvals.audit.ts`) owns every non-CLI `pending` and every
decision. Both call the same domain-free `src/audit/` emit/store. Later
surfaces (message traffic, tool calls, container lifecycle) attach the same way
— a new `*.audit.ts` adapter, no schema change.
## Steps
### 0. Pre-flight (idempotency)
The apply is safe to re-run; every step below is guarded. Skip to
**Enable** if all of these already hold:
- `src/audit/` exists
- `src/cli/resources/index.ts` contains `import './audit.js';`
- `src/cli/dispatch.ts` contains `export const dispatch = withAudit(dispatchInner);`
- `src/modules/approvals/index.ts` contains `import './approvals.audit.js';`
Before editing, verify the reach-in targets still exist: `src/cli/dispatch.ts`
must contain `export async function dispatch(` (or the already-applied
composition), `src/cli/resources/index.ts` must be the resource barrel (a list
of `import './<resource>.js';` lines), and `src/modules/approvals/index.ts`
must be the approvals barrel exposing the lifecycle observers
(`registerApprovalRequestedHandler` / `registerApprovalResolvedHandler` in
`./primitive.js`). If any has moved, stop and adapt rather than guessing.
### 1. Copy the payload
From the NanoClaw project root:
```bash
cp -R "${CLAUDE_SKILL_DIR}/add/src/." src/
```
What lands (mirrors of the destination paths):
- `src/audit/` — the domain-free leaf: event schema (`types.ts`), env config
(`config.ts`), redactor, NDJSON day-file store, emit seam, post-write hooks,
reader, boot/maintenance wiring, vocabulary — plus its tests.
- `src/cli/dispatch.audit.ts` — the CLI adapter: `withAudit` middleware and
the actor/origin/resource mapping (+ `dispatch.audit.test.ts`).
- `src/modules/approvals/approvals.audit.ts` — the approvals adapter: the two
lifecycle-observer subscriptions that emit `pending` and `approvals.decide`
(+ `approvals.audit.test.ts`).
- `src/cli/resources/audit.ts` — the read-only `ncl audit` resource.
- `src/audit-wiring.test.ts` — goes red if any of the three core edits below is
deleted or drifts.
### 2. Register the resource
Append to `src/cli/resources/index.ts` (skip if the line is already present):
```typescript
import './audit.js';
```
### 3. Compose the dispatch middleware
This is the skill's one functional reach-in, in `src/cli/dispatch.ts`. Three
small edits:
1. Add the import (next to the other `./` imports):
```typescript
import { withAudit } from './dispatch.audit.js';
```
2. Rename the dispatcher declaration — change
```typescript
export async function dispatch(
```
to
```typescript
async function dispatchInner(
```
3. Directly after that function's closing brace (before the
`registerApprovalHandler('cli_command', …)` block), add:
```typescript
// Audit middleware (installed by /add-audit): the exported dispatch is the
// wrapped function, so both transports and the approved replay below all
// pass the one composition.
export const dispatch = withAudit(dispatchInner);
```
The composition must live at the definition site (not at the import sites):
the approved-replay handler in the same file calls `dispatch(...)` too, and
only the wrapped export covers it. `src/audit-wiring.test.ts` asserts exactly
this shape via the TypeScript AST.
Loading `dispatch.audit.ts` also boots the audit log: on an enabled box it
asserts `data/audit/` is writable (refusing to start beats a silent audit
gap), runs the boot retention prune, and arms an unref'd maintenance timer.
(The approvals adapter in the next step boots it too — idempotent — so the
approval surface records even on a build that loads it first.)
### 4. Wire the approval observers
The approval-lifecycle increment's one reach-in, in the approvals barrel
`src/modules/approvals/index.ts`. Append a side-effect import (next to the
other module imports; skip if already present):
```typescript
// Approval-lifecycle audit observers (installed by /add-audit): importing the
// adapter registers its request/decision observers at boot.
import './approvals.audit.js';
```
Importing the adapter is what registers its two observers
(`registerApprovalRequestedHandler` / `registerApprovalResolvedHandler` from
`./primitive.js`) — the request/decision events for every non-CLI hold. The
barrel already loads at boot via `src/modules/index.ts`, and
`src/audit-wiring.test.ts` asserts the import is present. No `host-sweep.ts`
edit.
### 5. Enable
Add the two settings to `.env` (idempotent — overwrite or append):
```bash
grep -q '^AUDIT_ENABLED=' .env && sed -i.bak 's/^AUDIT_ENABLED=.*/AUDIT_ENABLED=true/' .env && rm -f .env.bak || echo 'AUDIT_ENABLED=true' >> .env
grep -q '^AUDIT_RETENTION_DAYS=' .env || echo 'AUDIT_RETENTION_DAYS=90' >> .env
```
`AUDIT_ENABLED` is the master switch — off (or absent) means `emitAuditEvent`
is a no-op and `data/audit/` is never created. `AUDIT_RETENTION_DAYS`: day
files strictly older than the horizon are hard-deleted (unlinked) at boot and
once per UTC day; `0` = keep forever; unset = 90.
### 6. Build and test
Run `build` before the tests — it's what catches a missed copy or a drifted
import path across the whole composed tree:
```bash
pnpm run build
pnpm exec vitest run src/audit src/cli/dispatch.audit.test.ts src/modules/approvals/approvals.audit.test.ts src/audit-wiring.test.ts
pnpm test
```
### 7. Restart the service
```bash
source setup/lib/install-slug.sh
launchctl kickstart -k gui/$(id -u)/$(launchd_label) # macOS
# systemctl --user restart $(systemd_unit) # Linux
```
### 8. Verify (runtime smoke)
```bash
ncl groups list # any command — this one is now an audit event
cat data/audit/$(date -u +%F).ndjson | tail -1
ncl audit list --limit 5
ncl audit list --outcome pending # what is waiting for approval right now
```
The last line of the day-file is the `groups.list` event you just caused. To
see the approval surface, drive a gated action (e.g. an agent asking to wire an
MCP server) and approve it, then `ncl audit list --correlation <approval-id>`
returns the full chain: `pending` → `approvals.decide` → the terminal event.
## Reading it back
```
ncl audit list [--actor <id>] [--action <name-or-prefix>] [--resource <id-or-type>]
[--outcome success|failure|denied|pending|approved|rejected]
[--since 7d|24h|30m|ISO] [--until …] [--correlation <approval-id>]
[--limit N] [--format ndjson]
```
Newest first, default limit 100. `--action groups.config` matches the whole
dotted subtree. `--format ndjson` streams the stored lines verbatim — pipe to
a file for SIEM import. On a disabled box the command errors with
`audit log is disabled — set AUDIT_ENABLED=true` rather than returning an
empty list that would read as "no actions happened".
The resource is deliberately **not** on the group-scope allowlist: audit
spans agent groups, so group-scoped agents are refused before the handler
(fails closed). Host callers and `cli_scope: global` agents only.
## Recording model, failure posture, exporters
- Events store WHO did WHICH action to WHAT target and the outcome — **never
raw argument values**. `details` carries the flag names that were passed plus
a small allowlist of governance-relevant enum fields (`role`, `mode`,
`session_mode`, `cli_scope`, `access`, `engage_mode`, `sender_scope`,
`provider`, `model`) echoed verbatim; every other value is name-only, and a
failure keeps the error *code*, never the free-text message. There is no value
redactor — a secret can't leak from a value that is never stored. Target
identifiers (ids, users, groups) surface structurally in `resources`.
- Fail-open + loud: a failed append is `log.error`'d and the audited action
proceeds — the emit brackets the dispatcher, so even a command that *throws*
still leaves a record. At boot, an enabled box refuses to start if
`data/audit/` isn't writable.
- In-process exporters register via `registerAuditHook` (from
`src/audit/index.js`) — post-write hooks that fire only after the local
append succeeds, so an external system can never be ahead of the source of
truth. Ship one as its own `/add-*` skill; declare `/add-audit` as its
dependency.
## Troubleshooting
- **Host won't start, banner names the audit directory** — `AUDIT_ENABLED=true`
but `data/audit/` isn't writable. Fix permissions or disable audit.
- **`audit log is disabled — set AUDIT_ENABLED=true`** — the read-back guard;
enable in `.env` and restart.
- **An agent gets "CLI access is scoped to this agent group" for `ncl audit`**
— by design; grant the group `cli_scope: global` only if it should read
cross-group history.
- **`pnpm test` on an enabled box adds a few events to today's day-file** —
expected: the base dispatch tests drive real dispatches. Harmless noise;
the audit test suites themselves write only to temp dirs or capture buffers.
## Removal
See [REMOVE.md](REMOVE.md) — reverses every change; existing day-files are
left in place (they're the operator's records).
@@ -1,152 +0,0 @@
/**
* Wiring tests for the /add-audit skill's two core edits — they go red if
* either edit is deleted or drifts:
*
* 1. src/cli/dispatch.ts — the exported dispatch must be the composed
* `withAudit(dispatchInner)` (AST check: only the definition-site
* composition covers both transports AND the in-module approved replay).
* 2. src/cli/resources/index.ts — the audit resource must register through
* the real barrel (behavior check), stay OFF the group-scope allowlist,
* and leave guard conformance clean.
*
* Plus the emit invariant the module's discipline rests on: emitAuditEvent
* appears only in src/audit/ and *.audit.ts adapter files.
*/
import fs from 'fs';
import path from 'path';
import ts from 'typescript';
import { describe, expect, it } from 'vitest';
// Same production barrels the guard conformance test loads (mirrors
// src/guard/conformance.test.ts).
import './cli/commands/index.js';
import './modules/index.js';
import './cli/delivery-action.js';
import './cli/dispatch.js';
import { commandGuard, GROUP_SCOPE_RESOURCES, lookup } from './cli/registry.js';
import { getApprovalHandler } from './modules/approvals/primitive.js';
import { listGuardedActions } from './guard/index.js';
describe('audit resource registration (barrel wiring)', () => {
it('registers audit-list through the real resource barrel', () => {
const cmd = lookup('audit-list');
expect(cmd).toBeDefined();
expect(cmd?.action).toBe('audit.list');
expect(cmd?.access).toBe('open');
});
it('derives a guard-catalog entry for the audit command', () => {
const guard = commandGuard('audit-list');
expect(guard.action).toBe('audit.list');
});
it('is NOT on the group-scope allowlist — group-scoped agents fail closed', () => {
expect(GROUP_SCOPE_RESOURCES.has('audit')).toBe(false);
});
it('leaves guard conformance clean with the audit resource registered', () => {
// The invariant guard conformance checks: every holding action pairs with a
// registered approve continuation. audit.list is `open` (never holds), so
// registering the resource can't introduce a gap — this pins that.
const dangling = listGuardedActions()
.filter((spec) => spec.grantActionName)
.filter((spec) => !getApprovalHandler(spec.grantActionName as string));
expect(dangling.map((s) => s.action)).toEqual([]);
});
});
describe('dispatch composition (AST wiring)', () => {
const source = fs.readFileSync(path.resolve('src/cli/dispatch.ts'), 'utf8');
const sf = ts.createSourceFile('dispatch.ts', source, ts.ScriptTarget.Latest, true);
const hasExportModifier = (node: ts.HasModifiers): boolean =>
(ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword);
let importsWithAudit = false;
let innerDeclaredUnexported = false;
let exportsWrappedDispatch = false;
let exportsUnwrappedDispatchFn = false;
sf.forEachChild((node) => {
if (
ts.isImportDeclaration(node) &&
ts.isStringLiteral(node.moduleSpecifier) &&
node.moduleSpecifier.text === './dispatch.audit.js'
) {
const named = node.importClause?.namedBindings;
if (named && ts.isNamedImports(named) && named.elements.some((e) => e.name.text === 'withAudit')) {
importsWithAudit = true;
}
}
if (ts.isFunctionDeclaration(node) && node.name?.text === 'dispatchInner') {
innerDeclaredUnexported = !hasExportModifier(node);
}
if (ts.isFunctionDeclaration(node) && node.name?.text === 'dispatch' && hasExportModifier(node)) {
exportsUnwrappedDispatchFn = true;
}
if (ts.isVariableStatement(node) && hasExportModifier(node)) {
for (const decl of node.declarationList.declarations) {
if (
ts.isIdentifier(decl.name) &&
decl.name.text === 'dispatch' &&
decl.initializer &&
ts.isCallExpression(decl.initializer) &&
ts.isIdentifier(decl.initializer.expression) &&
decl.initializer.expression.text === 'withAudit' &&
decl.initializer.arguments.length === 1 &&
ts.isIdentifier(decl.initializer.arguments[0]) &&
decl.initializer.arguments[0].text === 'dispatchInner'
) {
exportsWrappedDispatch = true;
}
}
}
});
it('imports withAudit from the audit adapter', () => {
expect(importsWithAudit).toBe(true);
});
it('keeps the inner dispatcher unexported (the guarded path is the only path)', () => {
expect(innerDeclaredUnexported).toBe(true);
});
it('exports dispatch as withAudit(dispatchInner)', () => {
expect(exportsWrappedDispatch).toBe(true);
expect(exportsUnwrappedDispatchFn).toBe(false);
});
});
describe('approvals adapter wiring', () => {
it('the approvals barrel imports the approval audit adapter', () => {
// The one reach-in the approval-lifecycle increment adds: importing the
// adapter is what registers its two observers at boot. Goes red if dropped.
const source = fs.readFileSync(path.resolve('src/modules/approvals/index.ts'), 'utf8');
expect(source).toMatch(/import\s+['"]\.\/approvals\.audit\.js['"];/);
});
});
describe('emit invariant', () => {
it('emitAuditEvent appears only in src/audit/ and *.audit.ts files', () => {
const srcRoot = path.resolve('src');
const offenders: string[] = [];
const walk = (dir: string): void => {
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) {
walk(full);
continue;
}
if (!entry.name.endsWith('.ts')) continue;
const rel = path.relative(srcRoot, full).split(path.sep).join('/');
if (rel === 'audit-wiring.test.ts') continue; // this file names the symbol
if (rel.startsWith('audit/')) continue;
if (/\.audit(\.test)?\.ts$/.test(rel)) continue;
if (fs.readFileSync(full, 'utf8').includes('emitAuditEvent')) offenders.push(rel);
}
};
walk(srcRoot);
expect(offenders).toEqual([]);
});
});
@@ -1,30 +0,0 @@
/**
* Audit env config (installed by /add-audit) — the two audit vars, read the
* same way src/config.ts reads every host setting (readEnvFile from .env,
* with process.env taking precedence). Kept audit-owned so installing the
* skill never edits core config.ts: the feature's whole footprint in core is
* the dispatch composition in dispatch.ts and the resource-barrel import.
*/
import { readEnvFile } from '../env.js';
const envConfig = readEnvFile(['AUDIT_ENABLED', 'AUDIT_RETENTION_DAYS']);
/**
* Master switch. Off by default — nothing is persisted (and data/audit/ is
* never created) until an operator sets AUDIT_ENABLED=true.
*/
export const AUDIT_ENABLED = (process.env.AUDIT_ENABLED || envConfig.AUDIT_ENABLED) === 'true';
/**
* Day-file retention horizon in days; consulted only when audit is enabled.
* Unset or unparseable → 90. An explicit 0 (or negative) = keep forever.
*/
function parseRetentionDays(raw: string | undefined): number {
if (raw === undefined || raw.trim() === '') return 90;
const n = Number.parseInt(raw, 10);
return Number.isNaN(n) ? 90 : n;
}
export const AUDIT_RETENTION_DAYS = parseRetentionDays(
process.env.AUDIT_RETENTION_DAYS || envConfig.AUDIT_RETENTION_DAYS,
);
@@ -1,45 +0,0 @@
/**
* The single emit seam. Adapters import emitAuditEvent from here directly
* (it is deliberately not re-exported by the barrel): only src/audit/ and
* `*.audit.ts` adapter files may call it — grep holds the invariant.
*
* The opt-in check lives here, so the whole feature switches at one point.
* Fail-open + loud: a failed append (or a throwing input thunk) is
* log.error'd and the audited action proceeds.
*/
import { randomUUID } from 'crypto';
import { log } from '../log.js';
import { AUDIT_ENABLED } from './config.js';
import { notifyAuditHooks } from './hooks.js';
import { appendAuditLine } from './store.js';
import type { AuditEvent, AuditEventInput } from './types.js';
export function emitAuditEvent(input: AuditEventInput | (() => AuditEventInput)): void {
if (!AUDIT_ENABLED) return; // The one opt-in check — the whole feature switches here.
try {
if (typeof input === 'function') input = input();
const event: AuditEvent = {
event_id: randomUUID(),
time: new Date().toISOString(),
schema_version: 1,
actor: { ...input.actor, email: null, user_id: null, group_ids: null },
origin: input.origin,
action: input.action,
resources: input.resources,
outcome: input.outcome,
correlation_id: input.correlationId ?? null,
// Adapters build `details` value-free (key names + an enum allowlist), so
// there is nothing to redact here — a secret can't leak from a value that
// is never stored. Written verbatim.
details: input.details ?? {},
};
const line = JSON.stringify(event);
appendAuditLine(line, event.time.slice(0, 10));
notifyAuditHooks(event, line);
// eslint-disable-next-line no-catch-all/no-catch-all -- fail-open is the posture: an audit failure must never take down the audited action
} catch (err) {
const action = typeof input === 'function' ? undefined : input.action;
log.error('Audit append failed — action proceeding (fail-open)', { action, err });
}
}
@@ -1,240 +0,0 @@
/**
* Post-write hook contract: hooks observe the LOG (fire only after a
* successful append, exported ⊆ written), failures are isolated everywhere,
* and the lifecycle (init/maintain/shutdown) behaves.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
const state = vi.hoisted(() => ({ enabled: true, appendThrows: false, appended: [] as string[] }));
vi.mock('../config.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../config.js')>();
return {
...actual,
DATA_DIR: '/tmp/nanoclaw-test-hooks-unused',
};
});
vi.mock('./config.js', () => ({
get AUDIT_ENABLED() {
return state.enabled;
},
AUDIT_RETENTION_DAYS: 90,
}));
vi.mock('./store.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('./store.js')>();
return {
...actual,
appendAuditLine: (line: string) => {
if (state.appendThrows) throw new Error('disk full');
state.appended.push(line);
},
};
});
vi.mock('../log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
let hooks: typeof import('./hooks.js');
let emit: typeof import('./emit.js');
let log: (typeof import('../log.js'))['log'];
beforeEach(async () => {
state.enabled = true;
state.appendThrows = false;
state.appended.length = 0;
vi.resetModules(); // fresh hook registry per test
hooks = await import('./hooks.js');
emit = await import('./emit.js');
log = (await import('../log.js')).log;
vi.clearAllMocks();
});
afterEach(() => {
vi.clearAllMocks();
});
const EVENT_INPUT = {
actor: { type: 'human' as const, id: 'host:test' },
origin: { transport: 'socket' as const },
action: 'groups.list',
resources: [{ type: 'agent_group' }],
outcome: 'success' as const,
details: { limit: 5 },
};
describe('post-write notification', () => {
it('calls a registered hook with the parsed event and the exact stored line', () => {
const seen: Array<{ event: import('./types.js').AuditEvent; line: string }> = [];
hooks.registerAuditHook({ name: 'demo', onEvent: (event, line) => seen.push({ event, line }) });
emit.emitAuditEvent(EVENT_INPUT);
expect(state.appended).toHaveLength(1);
expect(seen).toHaveLength(1);
expect(seen[0].line).toBe(state.appended[0]);
expect(JSON.parse(seen[0].line)).toEqual(seen[0].event);
expect(seen[0].event.action).toBe('groups.list');
});
it('does NOT call hooks when the local append fails — exported ⊆ written', () => {
const onEvent = vi.fn();
hooks.registerAuditHook({ name: 'demo', onEvent });
state.appendThrows = true;
expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow(); // action still proceeds
expect(onEvent).not.toHaveBeenCalled();
expect(log.error).toHaveBeenCalledWith(expect.stringContaining('Audit append failed'), expect.anything());
});
it('does NOT call hooks when audit is disabled', () => {
const onEvent = vi.fn();
hooks.registerAuditHook({ name: 'demo', onEvent });
state.enabled = false;
emit.emitAuditEvent(EVENT_INPUT);
expect(state.appended).toHaveLength(0);
expect(onEvent).not.toHaveBeenCalled();
});
it('isolates a throwing hook: the write survives, later hooks still run, the action proceeds', () => {
const second = vi.fn();
hooks.registerAuditHook({
name: 'broken',
onEvent: () => {
throw new Error('exporter exploded');
},
});
hooks.registerAuditHook({ name: 'healthy', onEvent: second });
expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow();
expect(state.appended).toHaveLength(1); // the log has the event regardless
expect(second).toHaveBeenCalledTimes(1);
expect(log.error).toHaveBeenCalledWith(
expect.stringContaining('Audit hook threw'),
expect.objectContaining({ hook: 'broken', action: 'groups.list' }),
);
});
});
describe('lifecycle', () => {
it('initAuditHooks surfaces a failing init as a fatal error naming the hook', () => {
hooks.registerAuditHook({ name: 'ok', onEvent: () => {}, init: vi.fn() });
hooks.registerAuditHook({
name: 'bad-boot',
onEvent: () => {},
init: () => {
throw new Error('no route to collector');
},
});
expect(() => hooks.initAuditHooks()).toThrow(/audit hook "bad-boot" failed to initialize.*no route/);
});
it('defers init() until boot for a hook registered before initAuditHooks', () => {
const init = vi.fn();
hooks.registerAuditHook({ name: 'early', onEvent: () => {}, init });
expect(init).not.toHaveBeenCalled(); // not yet — boot hasn't run
hooks.initAuditHooks();
expect(init).toHaveBeenCalledTimes(1); // exactly once, at boot
});
it('runs init() immediately for a hook registered after boot — import-order-insensitive', () => {
hooks.initAuditHooks(); // boot completes (no hooks yet)
const init = vi.fn();
const onEvent = vi.fn();
hooks.registerAuditHook({ name: 'late', onEvent, init });
// A module that loaded after the CLI adapter still gets its one-time init...
expect(init).toHaveBeenCalledTimes(1);
// ...and still receives events (onEvent already read the live array).
emit.emitAuditEvent(EVENT_INPUT);
expect(onEvent).toHaveBeenCalledTimes(1);
});
it('a hook registered after boot whose init throws is fatal, naming the hook', () => {
hooks.initAuditHooks();
expect(() =>
hooks.registerAuditHook({
name: 'late-bad',
onEvent: () => {},
init: () => {
throw new Error('no route to collector');
},
}),
).toThrow(/audit hook "late-bad" failed to initialize.*no route/);
});
it('maintainAuditHooks calls every maintain and isolates throws', () => {
const m1 = vi.fn(() => {
throw new Error('flush failed');
});
const m2 = vi.fn();
hooks.registerAuditHook({ name: 'a', onEvent: () => {}, maintain: m1 });
hooks.registerAuditHook({ name: 'b', onEvent: () => {}, maintain: m2 });
hooks.registerAuditHook({ name: 'c', onEvent: () => {} }); // no maintain — fine
expect(() => hooks.maintainAuditHooks()).not.toThrow();
expect(m1).toHaveBeenCalledTimes(1);
expect(m2).toHaveBeenCalledTimes(1);
expect(log.error).toHaveBeenCalledWith(
expect.stringContaining('maintenance failed'),
expect.objectContaining({ hook: 'a' }),
);
});
it('shutdownAuditHooks awaits async shutdowns and isolates throws', async () => {
const order: string[] = [];
hooks.registerAuditHook({
name: 'a',
onEvent: () => {},
shutdown: async () => {
await Promise.resolve();
order.push('a');
},
});
hooks.registerAuditHook({
name: 'b',
onEvent: () => {},
shutdown: () => {
throw new Error('handle already closed');
},
});
hooks.registerAuditHook({
name: 'c',
onEvent: () => {},
shutdown: () => {
order.push('c');
},
});
await hooks.shutdownAuditHooks();
expect(order).toEqual(['a', 'c']);
expect(log.error).toHaveBeenCalledWith(
expect.stringContaining('shutdown failed'),
expect.objectContaining({ hook: 'b' }),
);
});
it('maintainAudit skips hook maintenance when audit is disabled', async () => {
const init = await import('./init.js');
const maintain = vi.fn();
hooks.registerAuditHook({ name: 'a', onEvent: () => {}, maintain });
state.enabled = false;
init.maintainAudit();
expect(maintain).not.toHaveBeenCalled();
state.enabled = true;
init.maintainAudit();
expect(maintain).toHaveBeenCalledTimes(1);
});
});
@@ -1,111 +0,0 @@
/**
* Post-write audit hooks — the in-process extension seam.
*
* A hook observes the audit LOG, not the event stream: `onEvent` fires only
* after an event has been durably appended to the local day-file, so anything
* a hook exports is guaranteed to exist in the source of truth
* (exported ⊆ written). If the local append fails, hooks are not called —
* and a hook that misses events (crash, restart) catches up by reading the
* day-files, which is the at-least-once story.
*
* Registration follows the tree's observer idiom (registerApprovalResolvedHandler,
* registerResponseHandler, …): an in-tree or skill-installed module calls
* `registerAuditHook(...)` at import time — no core edits, and credentials or
* transport for an external system live in that module, never here.
*/
import { log } from '../log.js';
import type { AuditEvent } from './types.js';
export interface AuditHook {
/** Short identifier used in logs and lifecycle errors. */
name: string;
/**
* Called after a successful local append. `line` is the exact stored bytes
* (one NDJSON line, no trailing newline); `event` is the parsed record.
* MUST be fast and non-blocking — this runs on the audited action's call
* path. A real exporter buffers here and does its IO from `maintain`/its own
* timers. Throwing is tolerated: isolated and logged, never propagated.
*/
onEvent(event: AuditEvent, line: string): void;
/**
* One-time setup, called once when audit is enabled — at boot if the hook is
* already registered, else immediately on registration (so it fires exactly
* once regardless of import order). Throw = host refuses to start.
*/
init?(): void;
/** Periodic maintenance — called from the audit maintenance timer (enabled boxes only). */
maintain?(): void;
/** Graceful-shutdown hook (flush buffers, close handles). */
shutdown?(): void | Promise<void>;
}
const hooks: AuditHook[] = [];
let hooksInitialized = false;
export function registerAuditHook(hook: AuditHook): void {
hooks.push(hook);
// onEvent/maintain/shutdown all read the live array, so they pick a hook up
// whenever it registers. init() is the exception — it runs once at boot. If
// boot already ran (a hook whose module loaded after the CLI audit adapter),
// run its init() now; otherwise it would receive events but never its boot
// hook. Same throw-is-fatal posture as boot.
if (hooksInitialized) initOneHook(hook);
}
/** Fan out one written event to every hook, isolating failures per hook. */
export function notifyAuditHooks(event: AuditEvent, line: string): void {
for (const hook of hooks) {
try {
hook.onEvent(event, line);
// eslint-disable-next-line no-catch-all/no-catch-all -- isolation is the contract: one bad hook must not affect the log, other hooks, or the audited action
} catch (err) {
log.error('Audit hook threw — event is safely in the log', { hook: hook.name, action: event.action, err });
}
}
}
/** Boot lifecycle. A hook that can't start is a silent-export-gap risk — fatal. */
export function initAuditHooks(): void {
for (const hook of hooks) initOneHook(hook);
hooksInitialized = true;
}
/**
* Run one hook's boot init with the fatal-on-throw posture. Shared by the boot
* sweep and by a post-boot registration, so every hook gets exactly one init()
* regardless of the import order its module loaded in.
*/
function initOneHook(hook: AuditHook): void {
try {
hook.init?.();
} catch (err) {
throw new Error(
`audit hook "${hook.name}" failed to initialize: ${err instanceof Error ? err.message : String(err)}`,
{ cause: err },
);
}
}
/** Periodic lifecycle — maintenance, isolated per hook. */
export function maintainAuditHooks(): void {
for (const hook of hooks) {
try {
hook.maintain?.();
// eslint-disable-next-line no-catch-all/no-catch-all -- one hook's maintenance failure must not stop the others (or the timer)
} catch (err) {
log.error('Audit hook maintenance failed', { hook: hook.name, err });
}
}
}
/** Shutdown lifecycle — awaited by the host's graceful shutdown. */
export async function shutdownAuditHooks(): Promise<void> {
for (const hook of hooks) {
try {
await hook.shutdown?.();
// eslint-disable-next-line no-catch-all/no-catch-all -- shutdown must drain every hook even when one throws
} catch (err) {
log.error('Audit hook shutdown failed', { hook: hook.name, err });
}
}
}
@@ -1,17 +0,0 @@
/**
* Opt-in local audit log — installed by /add-audit.
*
* This directory is a domain-free leaf: the event schema, the emit seam, the
* store, the reader, post-write hooks, and shared vocabulary. What gets
* audited — and how each domain describes itself — lives in the domain-owned
* `*.audit.ts` adapter files next to the code they observe. Business logic
* contains zero audit calls.
*
* emitAuditEvent is deliberately NOT re-exported here: adapters import it
* from './emit.js' directly, and only src/audit/ + `*.audit.ts` may call it.
*/
export * from './types.js';
export { AUDIT_DIR } from './store.js';
export { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js';
export { initAuditLog, maintainAudit } from './init.js';
export { type AuditHook, registerAuditHook } from './hooks.js';
@@ -1,54 +0,0 @@
/**
* Boot-time audit wiring, self-contained (installed by /add-audit).
*
* initAuditLog() runs at module load of the CLI audit adapter
* (cli/dispatch.audit.ts) — i.e. during the host's barrel phase, before the
* CLI server or any delivery poll accepts work — because dispatch.ts, which
* both transports import, composes withAudit at module scope. When enabled:
* assert data/audit/ is writable (refusing to start beats running with a
* silent audit gap), run the boot prune, start the registered post-write
* hooks, and arm the maintenance timer.
*
* The timer owns the daily cadence in-module (checked hourly; the prune
* itself fires at most once per UTC day) so installing the skill touches
* dispatch.ts and the resource barrel only — no host-sweep edit. It is
* unref'd: it never keeps the process alive.
*/
import { log } from '../log.js';
import { onShutdown } from '../response-registry.js';
import { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js';
import { initAuditHooks, maintainAuditHooks, shutdownAuditHooks } from './hooks.js';
import { assertAuditWritable, AUDIT_DIR, markPrunedToday, pruneAuditLog, pruneAuditLogIfDue } from './store.js';
const MAINTENANCE_INTERVAL_MS = 60 * 60 * 1000;
let initialized = false;
export function initAuditLog(): void {
if (!AUDIT_ENABLED || initialized) return;
initialized = true;
try {
assertAuditWritable();
} catch (err) {
throw new Error(
`AUDIT_ENABLED=true but the audit directory is not writable: ${AUDIT_DIR} (${err instanceof Error ? err.message : String(err)})`,
{ cause: err },
);
}
pruneAuditLog();
markPrunedToday();
initAuditHooks(); // throw → boot fails, same posture as the writability assert
onShutdown(() => shutdownAuditHooks());
setInterval(maintainAudit, MAINTENANCE_INTERVAL_MS).unref();
log.info('Audit log enabled', { dir: AUDIT_DIR, retentionDays: AUDIT_RETENTION_DAYS });
}
/**
* One maintenance tick: retention prune (throttled to once per UTC day
* internally) plus every hook's periodic maintenance. No-op when audit is
* disabled. Exported so a future scheduler can drive it too.
*/
export function maintainAudit(): void {
pruneAuditLogIfDue();
if (AUDIT_ENABLED) maintainAuditHooks();
}
@@ -1,180 +0,0 @@
import fs from 'fs';
import os from 'os';
import path from 'path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
const state = vi.hoisted(() => ({ dataDir: '', enabled: true }));
vi.mock('../config.js', () => ({
get DATA_DIR() {
return state.dataDir;
},
}));
vi.mock('./config.js', () => ({
get AUDIT_ENABLED() {
return state.enabled;
},
AUDIT_RETENTION_DAYS: 90,
}));
vi.mock('../log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
let reader: typeof import('./reader.js');
let store: typeof import('./store.js');
beforeEach(async () => {
state.dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'audit-reader-'));
state.enabled = true;
vi.resetModules();
store = await import('./store.js');
reader = await import('./reader.js');
});
afterEach(() => {
fs.rmSync(state.dataDir, { recursive: true, force: true });
});
function dayString(daysAgo: number): string {
return store.utcDay(new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000));
}
function isoAt(daysAgo: number, tag: number): string {
const base = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000);
return `${base.toISOString().slice(0, 10)}T10:00:0${tag}.000Z`;
}
let seq = 0;
function seedEvent(daysAgo: number, overrides: Record<string, unknown> = {}): Record<string, unknown> {
seq += 1;
const event = {
event_id: `e-${seq}`,
time: isoAt(daysAgo, seq % 10),
schema_version: 1,
actor: { type: 'human', id: 'host:moshe', email: null, user_id: null, group_ids: null },
origin: { transport: 'socket' },
action: 'groups.list',
resources: [{ type: 'agent_group', id: 'ag-1' }],
outcome: 'success',
correlation_id: null,
details: {},
...overrides,
};
const dir = path.join(state.dataDir, 'audit');
fs.mkdirSync(dir, { recursive: true });
fs.appendFileSync(path.join(dir, `${dayString(daysAgo)}.ndjson`), JSON.stringify(event) + '\n');
return event;
}
describe('listAuditEvents', () => {
it('throws the disabled error rather than returning an empty list', () => {
state.enabled = false;
expect(() => reader.listAuditEvents({})).toThrow('audit log is disabled — set AUDIT_ENABLED=true');
});
it('returns flat rows newest-first across day-files, honoring --limit', () => {
seedEvent(2, { event_id: 'old' });
seedEvent(1, { event_id: 'mid-a' });
seedEvent(1, { event_id: 'mid-b' });
seedEvent(0, { event_id: 'new' });
const rows = reader.listAuditEvents({}) as Array<Record<string, unknown>>;
expect(rows.map((r) => r.event_id)).toEqual(['new', 'mid-b', 'mid-a', 'old']);
expect(rows[0]).toMatchObject({ actor: 'host:moshe', action: 'groups.list', outcome: 'success' });
expect(rows[0].resources).toBe('agent_group:ag-1');
const limited = reader.listAuditEvents({ limit: 2 }) as Array<Record<string, unknown>>;
expect(limited.map((r) => r.event_id)).toEqual(['new', 'mid-b']);
});
it('filters by actor, outcome, correlation, and resource (id or type)', () => {
seedEvent(0, { event_id: 'a', actor: { type: 'agent', id: 'ag-1' }, outcome: 'denied' });
seedEvent(0, { event_id: 'b', correlation_id: 'appr-9', resources: [{ type: 'approval', id: 'appr-9' }] });
seedEvent(0, { event_id: 'c', resources: [{ type: 'user', id: 'slack:U1' }] });
expect((reader.listAuditEvents({ actor: 'ag-1' }) as unknown[]).length).toBe(1);
expect((reader.listAuditEvents({ outcome: 'denied' }) as unknown[]).length).toBe(1);
expect((reader.listAuditEvents({ correlation: 'appr-9' }) as unknown[]).length).toBe(1);
expect((reader.listAuditEvents({ resource: 'slack:U1' }) as unknown[]).length).toBe(1);
expect((reader.listAuditEvents({ resource: 'approval' }) as unknown[]).length).toBe(1);
});
it('matches actions exactly or by dotted prefix', () => {
seedEvent(0, { event_id: 'cfg', action: 'groups.config.add-mcp-server' });
seedEvent(0, { event_id: 'list', action: 'groups.list' });
seedEvent(0, { event_id: 'other', action: 'sessions.list' });
expect((reader.listAuditEvents({ action: 'groups' }) as unknown[]).length).toBe(2);
expect((reader.listAuditEvents({ action: 'groups.config' }) as unknown[]).length).toBe(1);
expect((reader.listAuditEvents({ action: 'groups.list' }) as unknown[]).length).toBe(1);
// Prefix means dotted segments, not substrings.
expect((reader.listAuditEvents({ action: 'group' }) as unknown[]).length).toBe(0);
});
it('applies --since/--until with relative and ISO forms', () => {
seedEvent(5, { event_id: 'old' });
seedEvent(0, { event_id: 'recent' });
const relative = reader.listAuditEvents({ since: '2d' }) as Array<Record<string, unknown>>;
expect(relative.map((r) => r.event_id)).toEqual(['recent']);
const iso = reader.listAuditEvents({ until: dayString(2) }) as Array<Record<string, unknown>>;
expect(iso.map((r) => r.event_id)).toEqual(['old']);
expect(() => reader.listAuditEvents({ since: 'yesterdayish' })).toThrow('invalid --since');
});
it('--format ndjson returns the stored lines verbatim', () => {
const seeded = seedEvent(0, { event_id: 'x1' });
const out = reader.listAuditEvents({ format: 'ndjson' });
expect(typeof out).toBe('string');
expect(JSON.parse(out as string)).toEqual(seeded);
expect(() => reader.listAuditEvents({ format: 'csv' })).toThrow('invalid --format');
});
it('--format ndjson exports every event in the window (no silent 100-row cap)', () => {
for (let i = 0; i < 150; i++) seedEvent(0, { event_id: `e-${i}` });
const ndjson = reader.listAuditEvents({ format: 'ndjson' }) as string;
expect(ndjson.split('\n').length).toBe(150);
// The table view still caps at the default for readability.
expect((reader.listAuditEvents({}) as unknown[]).length).toBe(100);
});
it('honors an explicit --limit 0 as zero rows, not the default', () => {
seedEvent(0, { event_id: 'a' });
expect(reader.listAuditEvents({ limit: 0 })).toEqual([]);
});
it('rejects a negative or non-integer --limit', () => {
expect(() => reader.listAuditEvents({ limit: -1 })).toThrow('invalid --limit');
expect(() => reader.listAuditEvents({ limit: 'lots' })).toThrow('invalid --limit');
});
it('parses a timezone-less --since/--until datetime as UTC, not local', () => {
expect(reader.parseTimeFlag('2026-07-12T00:00:00', '--since')).toBe(Date.parse('2026-07-12T00:00:00Z'));
// Date-only already parses UTC; an explicit offset is respected as-is.
expect(reader.parseTimeFlag('2026-07-12', '--since')).toBe(Date.parse('2026-07-12'));
expect(reader.parseTimeFlag('2026-07-12T00:00:00+02:00', '--until')).toBe(
Date.parse('2026-07-12T00:00:00+02:00'),
);
});
it('skips malformed stored lines and still returns the rest', () => {
seedEvent(0, { event_id: 'good' });
fs.appendFileSync(path.join(state.dataDir, 'audit', `${dayString(0)}.ndjson`), 'not-json\n');
const rows = reader.listAuditEvents({}) as Array<Record<string, unknown>>;
expect(rows.map((r) => r.event_id)).toEqual(['good']);
});
it('rejects an unknown --outcome value', () => {
expect(() => reader.listAuditEvents({ outcome: 'meh' })).toThrow('invalid --outcome');
});
it('returns empty when nothing has been recorded yet', () => {
expect(reader.listAuditEvents({})).toEqual([]);
});
});
@@ -1,178 +0,0 @@
/**
* Read-back for `ncl audit list` — a newest-first stream-scan over the
* day-files. No index: fine at v1 volume, and adding one later doesn't change
* the store. NDJSON export returns the stored lines verbatim.
*/
import fs from 'fs';
import path from 'path';
import { log } from '../log.js';
import { AUDIT_ENABLED } from './config.js';
import { AUDIT_DIR, utcDay } from './store.js';
import type { AuditEvent, AuditOutcome } from './types.js';
const OUTCOMES: ReadonlySet<string> = new Set(['success', 'failure', 'denied', 'pending', 'approved', 'rejected']);
const DAY_FILE_RE = /^(\d{4}-\d{2}-\d{2})\.ndjson$/;
const DEFAULT_LIMIT = 100;
export interface AuditQuery {
actor?: string;
/** Exact action or dotted prefix (`groups` matches `groups.config.update`). */
action?: string;
/** Matches any resources[] entry by id or by type. */
resource?: string;
outcome?: AuditOutcome;
sinceMs?: number;
untilMs?: number;
correlation?: string;
limit: number;
}
/** `7d` / `24h` / `30m` relative to now, or an ISO date/datetime (UTC). */
export function parseTimeFlag(value: string, flag: string): number {
const rel = /^(\d+)([dhm])$/.exec(value);
if (rel) {
const n = Number(rel[1]);
const unitMs = rel[2] === 'd' ? 86_400_000 : rel[2] === 'h' ? 3_600_000 : 60_000;
return Date.now() - n * unitMs;
}
// A timezone-less ISO *datetime* parses as LOCAL per spec, but --since/--until
// are documented UTC; append 'Z' so a bare datetime reads as UTC. Date-only
// forms already parse as UTC, and an explicit offset (`+02:00`/`Z`) is left as-is.
const normalized = /^\d{4}-\d{2}-\d{2}T[\d.:]+$/.test(value) ? `${value}Z` : value;
const abs = Date.parse(normalized);
if (!Number.isNaN(abs)) return abs;
throw new Error(`invalid ${flag} value "${value}" — use e.g. 7d, 24h, 30m, or an ISO date`);
}
/** Newest first across files and within each file, up to q.limit. */
export function queryAuditEvents(q: AuditQuery): { events: AuditEvent[]; lines: string[] } {
const events: AuditEvent[] = [];
const lines: string[] = [];
let malformed = 0;
for (const { day, file } of dayFilesNewestFirst()) {
if (events.length >= q.limit) break;
// Whole-day skip: a file can't match a window its day lies outside.
if (q.sinceMs !== undefined && day < utcDay(new Date(q.sinceMs))) continue;
if (q.untilMs !== undefined && day > utcDay(new Date(q.untilMs))) continue;
let content: string;
try {
content = fs.readFileSync(file, 'utf8');
// eslint-disable-next-line no-catch-all/no-catch-all -- a torn/pruned day-file must not fail the whole query
} catch (err) {
log.warn('Audit reader failed to read day-file', { file, err });
continue;
}
const fileLines = content.split('\n').filter((l) => l.trim() !== '');
// Lines within a file are chronological — walk backwards for newest-first.
for (let i = fileLines.length - 1; i >= 0 && events.length < q.limit; i--) {
let event: AuditEvent;
try {
event = JSON.parse(fileLines[i]) as AuditEvent;
// eslint-disable-next-line no-catch-all/no-catch-all -- malformed stored lines are skipped (counted + warned below)
} catch {
malformed++;
continue;
}
if (!matches(event, q)) continue;
events.push(event);
lines.push(fileLines[i]);
}
}
if (malformed > 0) {
log.warn('Audit reader skipped malformed lines', { malformed });
}
return { events, lines };
}
function dayFilesNewestFirst(): Array<{ day: string; file: string }> {
let entries: string[];
try {
entries = fs.readdirSync(AUDIT_DIR);
// eslint-disable-next-line no-catch-all/no-catch-all -- no audit dir yet means no events, not an error
} catch {
return [];
}
return entries
.map((e) => DAY_FILE_RE.exec(e))
.filter((m): m is RegExpExecArray => m !== null)
.map((m) => ({ day: m[1], file: path.join(AUDIT_DIR, m[0]) }))
.sort((a, b) => (a.day < b.day ? 1 : -1));
}
function matches(event: AuditEvent, q: AuditQuery): boolean {
if (q.actor !== undefined && event.actor?.id !== q.actor) return false;
if (q.action !== undefined && event.action !== q.action && !event.action?.startsWith(q.action + '.')) return false;
if (q.outcome !== undefined && event.outcome !== q.outcome) return false;
if (q.correlation !== undefined && event.correlation_id !== q.correlation) return false;
if (q.resource !== undefined) {
const hit = (event.resources ?? []).some((r) => r.id === q.resource || r.type === q.resource);
if (!hit) return false;
}
const t = Date.parse(event.time ?? '');
if (q.sinceMs !== undefined && !(t >= q.sinceMs)) return false;
if (q.untilMs !== undefined && !(t <= q.untilMs)) return false;
return true;
}
/**
* `ncl audit list` handler. Disabled → an explicit error: an empty list would
* read as "no actions happened", which is a different truth than "not
* recording". `--format ndjson` returns the stored lines verbatim (the human
* formatter passes strings through); default returns flat rows for the table.
*/
export function listAuditEvents(args: Record<string, unknown>): string | Array<Record<string, unknown>> {
if (!AUDIT_ENABLED) {
throw new Error('audit log is disabled — set AUDIT_ENABLED=true');
}
const format = args.format !== undefined ? String(args.format) : '';
if (format && format !== 'ndjson') {
throw new Error(`invalid --format "${format}" — only "ndjson" is supported`);
}
const outcome = args.outcome !== undefined ? String(args.outcome) : undefined;
if (outcome !== undefined && !OUTCOMES.has(outcome)) {
throw new Error(`invalid --outcome "${outcome}" — one of: ${[...OUTCOMES].join(', ')}`);
}
// An explicit --limit is honored exactly (0 means 0, not "default"). Absent,
// the table view caps at DEFAULT_LIMIT for readability, but ndjson — the SIEM
// export path — is bounded only by --since/--until, never silently truncated.
let limit: number;
if (args.limit !== undefined) {
const n = Number(args.limit);
if (!Number.isInteger(n) || n < 0) {
throw new Error(`invalid --limit "${String(args.limit)}" — use a non-negative integer`);
}
limit = n;
} else {
limit = format === 'ndjson' ? Infinity : DEFAULT_LIMIT;
}
const q: AuditQuery = {
actor: args.actor !== undefined ? String(args.actor) : undefined,
action: args.action !== undefined ? String(args.action) : undefined,
resource: args.resource !== undefined ? String(args.resource) : undefined,
outcome: outcome as AuditOutcome | undefined,
correlation: args.correlation !== undefined ? String(args.correlation) : undefined,
sinceMs: args.since !== undefined ? parseTimeFlag(String(args.since), '--since') : undefined,
untilMs: args.until !== undefined ? parseTimeFlag(String(args.until), '--until') : undefined,
limit,
};
const { events, lines } = queryAuditEvents(q);
if (format === 'ndjson') return lines.join('\n');
return events.map((e) => ({
time: e.time,
actor: e.actor?.id ?? '',
action: e.action,
resources: (e.resources ?? []).map((r) => (r.id ? `${r.type}:${r.id}` : r.type)).join(' '),
outcome: e.outcome,
correlation: e.correlation_id ?? '',
event_id: e.event_id,
}));
}
@@ -1,197 +0,0 @@
import fs from 'fs';
import os from 'os';
import path from 'path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
// Core config (DATA_DIR) and the audit config (toggles) are mocked so
// store/emit resolve a per-test temp DATA_DIR and audit switches. Getters
// keep the values live across vi.resetModules().
const state = vi.hoisted(() => ({ dataDir: '', enabled: true, retention: 90 }));
vi.mock('../config.js', () => ({
get DATA_DIR() {
return state.dataDir;
},
}));
vi.mock('./config.js', () => ({
get AUDIT_ENABLED() {
return state.enabled;
},
get AUDIT_RETENTION_DAYS() {
return state.retention;
},
}));
vi.mock('../log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
let store: typeof import('./store.js');
let emit: typeof import('./emit.js');
let log: (typeof import('../log.js'))['log'];
beforeEach(async () => {
state.dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'audit-store-'));
state.enabled = true;
state.retention = 90;
vi.resetModules();
store = await import('./store.js');
emit = await import('./emit.js');
log = (await import('../log.js')).log;
vi.clearAllMocks();
});
afterEach(() => {
fs.chmodSync(path.join(state.dataDir), 0o700);
const auditDir = path.join(state.dataDir, 'audit');
if (fs.existsSync(auditDir)) fs.chmodSync(auditDir, 0o700);
fs.rmSync(state.dataDir, { recursive: true, force: true });
});
function auditDir(): string {
return path.join(state.dataDir, 'audit');
}
function writeDayFile(day: string, lines = 1): void {
fs.mkdirSync(auditDir(), { recursive: true });
fs.writeFileSync(path.join(auditDir(), `${day}.ndjson`), '{"x":1}\n'.repeat(lines));
}
function dayString(daysAgo: number): string {
return store.utcDay(new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000));
}
const EVENT_INPUT = {
actor: { type: 'human' as const, id: 'host:test' },
origin: { transport: 'socket' as const },
action: 'groups.list',
resources: [{ type: 'agent_group' }],
outcome: 'success' as const,
};
describe('appendAuditLine', () => {
it("appends one line to today's UTC day-file, creating the directory lazily", () => {
expect(fs.existsSync(auditDir())).toBe(false);
store.appendAuditLine('{"a":1}');
store.appendAuditLine('{"b":2}');
const file = path.join(auditDir(), `${store.utcDay()}.ndjson`);
expect(fs.readFileSync(file, 'utf8')).toBe('{"a":1}\n{"b":2}\n');
});
it('buckets by the given event day, not wall-clock at append', () => {
store.appendAuditLine('{"c":3}', '2020-01-02');
expect(fs.readFileSync(path.join(auditDir(), '2020-01-02.ndjson'), 'utf8')).toBe('{"c":3}\n');
// Today's file is untouched — the line filed under its own day.
expect(fs.existsSync(path.join(auditDir(), `${store.utcDay()}.ndjson`))).toBe(false);
});
});
describe('emitAuditEvent', () => {
it('writes a schema_version-1 record with envelope fields and null enrichment', () => {
emit.emitAuditEvent({ ...EVENT_INPUT, details: { limit: 100 } });
const file = path.join(auditDir(), `${store.utcDay()}.ndjson`);
const record = JSON.parse(fs.readFileSync(file, 'utf8').trim());
expect(record).toMatchObject({
schema_version: 1,
actor: { type: 'human', id: 'host:test', email: null, user_id: null, group_ids: null },
origin: { transport: 'socket' },
action: 'groups.list',
outcome: 'success',
correlation_id: null,
details: { limit: 100 },
});
expect(record.event_id).toMatch(/^[0-9a-f-]{36}$/);
expect(new Date(record.time).toISOString()).toBe(record.time);
});
it('stores details verbatim — the emit seam performs no value transformation', () => {
// emit is domain-free: adapters build `details` value-free, so there is
// nothing to redact here. Whatever is passed is written as-is.
emit.emitAuditEvent({ ...EVENT_INPUT, details: { args: ['role'], role: 'admin' } });
const file = path.join(auditDir(), `${store.utcDay()}.ndjson`);
const record = JSON.parse(fs.readFileSync(file, 'utf8').trim());
expect(record.details).toEqual({ args: ['role'], role: 'admin' });
});
it('is a no-op when audit is disabled — the directory is never created', () => {
state.enabled = false;
emit.emitAuditEvent(EVENT_INPUT);
expect(fs.existsSync(auditDir())).toBe(false);
});
it('fails open and loud when the append fails', () => {
fs.mkdirSync(auditDir(), { recursive: true });
fs.chmodSync(auditDir(), 0o500);
expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow();
expect(log.error).toHaveBeenCalledWith(
expect.stringContaining('Audit append failed'),
expect.objectContaining({ action: 'groups.list' }),
);
});
});
describe('assertAuditWritable', () => {
it('creates the directory and probes it with a zero-byte append', () => {
store.assertAuditWritable();
expect(fs.existsSync(path.join(auditDir(), `${store.utcDay()}.ndjson`))).toBe(true);
});
it('throws when the directory is not writable', () => {
fs.mkdirSync(auditDir(), { recursive: true });
fs.chmodSync(auditDir(), 0o500);
expect(() => store.assertAuditWritable()).toThrow();
});
});
describe('pruneAuditLog', () => {
it('unlinks only day-files strictly older than the horizon', () => {
writeDayFile(dayString(100));
writeDayFile(dayString(91));
writeDayFile(dayString(90));
writeDayFile(dayString(1));
fs.writeFileSync(path.join(auditDir(), 'not-a-day-file.txt'), 'keep');
fs.writeFileSync(path.join(auditDir(), '2026-01-01.ndjson.bak'), 'keep');
store.pruneAuditLog(90);
const left = fs.readdirSync(auditDir()).sort();
expect(left).toEqual(
[`${dayString(90)}.ndjson`, `${dayString(1)}.ndjson`, '2026-01-01.ndjson.bak', 'not-a-day-file.txt'].sort(),
);
});
it('keeps forever when retention is 0', () => {
writeDayFile(dayString(400));
store.pruneAuditLog(0);
expect(fs.existsSync(path.join(auditDir(), `${dayString(400)}.ndjson`))).toBe(true);
});
it('is a no-op when the audit directory does not exist', () => {
expect(() => store.pruneAuditLog(90)).not.toThrow();
});
});
describe('pruneAuditLogIfDue', () => {
it('prunes at most once per UTC day', () => {
writeDayFile(dayString(100));
store.pruneAuditLogIfDue();
expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(false);
writeDayFile(dayString(100));
store.pruneAuditLogIfDue();
expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true);
});
it('does nothing when audit is disabled', () => {
state.enabled = false;
writeDayFile(dayString(100));
store.pruneAuditLogIfDue();
expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true);
});
it('does nothing after markPrunedToday until the day rolls over', () => {
store.markPrunedToday();
writeDayFile(dayString(100));
store.pruneAuditLogIfDue();
expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true);
});
});
@@ -1,87 +0,0 @@
/**
* Day-file store — daily NDJSON files under data/audit/, host process is the
* single writer. Append-only is structural: nothing here can update a line,
* and retention is unlinking whole files (a literal hard delete). Both
* transports converge host-side, so single-writer holds by construction.
*
* This module throws on fs failure; the fail-open posture (log.error, action
* proceeds) lives in emit.ts, and the boot-time strictness (refuse to start)
* lives in init.ts.
*/
import fs from 'fs';
import path from 'path';
import { DATA_DIR } from '../config.js';
import { log } from '../log.js';
import { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js';
export const AUDIT_DIR = path.join(DATA_DIR, 'audit');
const DAY_FILE_RE = /^(\d{4}-\d{2}-\d{2})\.ndjson$/;
const DAY_MS = 24 * 60 * 60 * 1000;
export function utcDay(d: Date = new Date()): string {
return d.toISOString().slice(0, 10);
}
export function dayFilePath(day: string): string {
return path.join(AUDIT_DIR, `${day}.ndjson`);
}
/**
* The single append point. `day` buckets the line by the EVENT's own UTC day
* (emit passes it from event.time), not wall-clock at append — so a line whose
* emit crossed a UTC-midnight tick still files under its timestamp's day, and a
* bounded reader query can't whole-day-skip it. Throws on fs failure —
* emitAuditEvent catches.
*/
export function appendAuditLine(line: string, day: string = utcDay()): void {
fs.mkdirSync(AUDIT_DIR, { recursive: true });
fs.appendFileSync(dayFilePath(day), line + '\n');
}
/** Boot-time writability assert — a zero-byte append is a true write probe. */
export function assertAuditWritable(): void {
fs.mkdirSync(AUDIT_DIR, { recursive: true });
fs.appendFileSync(dayFilePath(utcDay()), '');
}
/** Unlink day-files strictly older than (today UTC retentionDays). 0 or negative = keep forever. */
export function pruneAuditLog(retentionDays: number = AUDIT_RETENTION_DAYS): void {
if (retentionDays <= 0) return;
let entries: string[];
try {
entries = fs.readdirSync(AUDIT_DIR);
// eslint-disable-next-line no-catch-all/no-catch-all -- no audit dir yet means nothing to prune, not an error
} catch {
return;
}
const horizon = utcDay(new Date(Date.now() - retentionDays * DAY_MS));
let pruned = 0;
for (const entry of entries) {
const m = DAY_FILE_RE.exec(entry);
if (!m || m[1] >= horizon) continue;
try {
fs.unlinkSync(path.join(AUDIT_DIR, entry));
pruned++;
// eslint-disable-next-line no-catch-all/no-catch-all -- one stuck file must not stop the prune of the others
} catch (err) {
log.error('Audit prune failed to unlink day-file', { file: entry, err });
}
}
if (pruned > 0) log.info('Audit retention pruned day-files', { pruned, retentionDays });
}
let lastPruneDay: string | null = null;
/** Retention prune, throttled to once per UTC day — safe to call every tick. */
export function pruneAuditLogIfDue(): void {
if (!AUDIT_ENABLED || AUDIT_RETENTION_DAYS <= 0) return;
const today = utcDay();
if (lastPruneDay === today) return;
lastPruneDay = today;
pruneAuditLog();
}
export function markPrunedToday(): void {
lastPruneDay = utcDay();
}
@@ -1,57 +0,0 @@
/**
* Canonical audit event — one flat, SIEM-shaped record per action.
*
* Fields are chosen so they project losslessly onto OCSF and Elastic ECS
* (the two schemas SIEMs converge on); the store keeps the neutral shape and
* a future forwarder pays the translation once, at the edge. `schema_version`
* covers evolution — additive changes only within a version.
*/
export type AuditActorType = 'human' | 'agent' | 'system';
export interface AuditActor {
type: AuditActorType;
/** `host:<os-user>` | `<channel>:<handle>` | agent group id | `host` (system). */
id: string;
}
export interface AuditOrigin {
transport: 'socket' | 'container' | 'channel';
session_id?: string;
messaging_group_id?: string;
channel?: string;
}
export interface AuditResource {
type: string;
/** Omitted when only the attempted type is known (e.g. a denied list). */
id?: string;
}
export type AuditOutcome = 'success' | 'failure' | 'denied' | 'pending' | 'approved' | 'rejected';
/** What emit sites provide. Envelope fields are stamped by emitAuditEvent. */
export interface AuditEventInput {
actor: AuditActor;
origin: AuditOrigin;
/** Dotted namespaced verb, e.g. `groups.config.add-mcp-server`. */
action: string;
resources: AuditResource[];
outcome: AuditOutcome;
/** The approval id on gated chains; null/omitted otherwise. */
correlationId?: string | null;
details?: Record<string, unknown>;
}
export interface AuditEvent {
event_id: string;
time: string;
schema_version: 1;
actor: AuditActor & { email: null; user_id: null; group_ids: null };
origin: AuditOrigin;
action: string;
resources: AuditResource[];
outcome: AuditOutcome;
correlation_id: string | null;
details: Record<string, unknown>;
}
@@ -1,73 +0,0 @@
/**
* Domain-free event vocabulary — actor and origin constructors shared by the
* domain-owned `*.audit.ts` adapters (the CLI adapter today; approval and
* channel adapters attach here in later increments). Pure derivation; nothing
* here writes the log.
*
* Leaf rule: this module (like the rest of src/audit/) may depend on node,
* config/log, shared types, and the db read layer — never on src/cli/* or
* src/modules/*. Domain-specific mapping (CLI resources, approval payloads)
* lives in the adapter file of the domain that owns it.
*/
import os from 'os';
import { getMessagingGroup } from '../db/messaging-groups.js';
import type { AuditOrigin } from './types.js';
/**
* Host callers stamp `host:<install user>` daemon-side: the ncl socket is
* 0600 and owned by the install user, so the identity is accurate by
* construction without peer credentials.
*/
export function hostUser(): string {
try {
return os.userInfo().username;
// eslint-disable-next-line no-catch-all/no-catch-all -- os.userInfo throws on exotic hosts; a fallback actor id beats no audit event
} catch {
return process.env.USER || 'unknown';
}
}
export function containerOrigin(sessionId: string, messagingGroupId: string | null): AuditOrigin {
const origin: AuditOrigin = { transport: 'container', session_id: sessionId };
if (messagingGroupId) {
origin.messaging_group_id = messagingGroupId;
const channel = getMessagingGroup(messagingGroupId)?.channel_type;
if (channel) origin.channel = channel;
}
return origin;
}
/**
* An approval decision answered on a chat platform (the approver clicked a
* card). `channelType` is the platform the click came from.
*/
export function channelOrigin(channelType: string | null): AuditOrigin {
return channelType ? { transport: 'channel', channel: channelType } : { transport: 'channel' };
}
/** Channel of a namespaced `<channel>:<handle>` id, or null if unprefixed. */
export function channelOf(namespacedUserId: string): string | null {
const i = namespacedUserId.indexOf(':');
return i > 0 ? namespacedUserId.slice(0, i) : null;
}
/**
* Dotted governance name for a hold's action, matching the guard catalog
* (agents.create, self_mod.*, a2a.send, senders.admit, channels.register).
* An unmapped action falls back to its raw name so a new gated surface still
* records — uncatalogued, never dropped.
*/
const APPROVAL_ACTION_DOTTED: Record<string, string> = {
create_agent: 'agents.create',
install_packages: 'self_mod.install_packages',
add_mcp_server: 'self_mod.add_mcp_server',
a2a_message_gate: 'a2a.send',
sender_admit: 'senders.admit',
channel_registration: 'channels.register',
onecli_credential: 'onecli.credential.use',
};
export function approvalActionName(action: string): string {
return APPROVAL_ACTION_DOTTED[action] ?? action;
}
@@ -1,363 +0,0 @@
/**
* Audit middleware behavior of the exported dispatch — what gets recorded,
* for whom, and how gated chains correlate. Drives the real wrapped dispatch
* (real registry, real guard); audit is force-enabled and the store's append
* is captured. DB reads and approval delivery are mocked.
*
* Recording model under test: the log stores arg KEY NAMES plus a small
* allowlist of safe enum values — never raw argument values — so a
* secret-bearing arg leaves only its key behind.
*/
import os from 'os';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { PendingApproval } from '../types.js';
const appended = vi.hoisted(() => ({ lines: [] as string[] }));
const pendingRows = vi.hoisted(() => ({ rows: [] as unknown[] }));
vi.mock('../audit/config.js', () => ({
AUDIT_ENABLED: true,
AUDIT_RETENTION_DAYS: 90,
}));
// Neutralize the adapter's module-scope boot (writability assert, prune,
// maintenance timer) — the middleware is the unit under test here.
vi.mock('../audit/init.js', () => ({
initAuditLog: vi.fn(),
maintainAudit: vi.fn(),
}));
vi.mock('../audit/store.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../audit/store.js')>();
return {
...actual,
appendAuditLine: (line: string) => {
appended.lines.push(line);
},
};
});
vi.mock('../log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
const mockGetContainerConfig = vi.fn();
vi.mock('../db/container-configs.js', () => ({
getContainerConfig: (...args: unknown[]) => mockGetContainerConfig(...args),
}));
vi.mock('../db/agent-groups.js', () => ({
getAgentGroup: vi.fn(() => ({ id: 'g1', name: 'Group One' })),
}));
const mockGetPendingApproval = vi.fn();
vi.mock('../db/sessions.js', () => ({
getSession: vi.fn(() => ({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' })),
getPendingApproval: (...args: unknown[]) => mockGetPendingApproval(...args),
getPendingApprovalsByAction: () => pendingRows.rows,
}));
vi.mock('../db/messaging-groups.js', () => ({
getMessagingGroup: vi.fn(() => ({ channel_type: 'slack' })),
}));
const mockGetResource = vi.fn();
vi.mock('./crud.js', () => ({
getResource: (...args: unknown[]) => mockGetResource(...args),
}));
const mockRequestApproval = vi.fn();
vi.mock('../modules/approvals/index.js', () => ({
registerApprovalHandler: vi.fn(),
requestApproval: (...args: unknown[]) => mockRequestApproval(...args),
}));
import { register } from './registry.js';
register({
name: 'groups-test',
description: 'echo command on the groups resource',
action: 'groups.test',
resource: 'groups',
access: 'open',
parseArgs: (raw) => raw,
handler: async (args) => ({ echo: args }),
});
register({
name: 'groups-get',
description: 'echo command for dash-joined id resolution',
action: 'groups.get',
resource: 'groups',
access: 'open',
parseArgs: (raw) => raw,
handler: async (args) => ({ echo: args }),
});
register({
name: 'wirings-list',
description: 'not on the group-scope allowlist',
action: 'wirings.list',
resource: 'wirings',
access: 'open',
parseArgs: (raw) => raw,
handler: async () => [],
});
register({
name: 'groups-fail',
description: 'handler that throws',
action: 'groups.fail',
resource: 'groups',
access: 'open',
parseArgs: (raw) => raw,
handler: async () => {
throw new Error('boom');
},
});
register({
name: 'groups-gated',
description: 'approval-gated command',
action: 'groups.gated',
resource: 'groups',
access: 'approval',
parseArgs: (raw) => raw,
handler: async () => 'ran',
});
register({
name: 'roles-grant',
description: 'command carrying an allowlisted --role value',
action: 'roles.grant',
resource: 'roles',
access: 'open',
parseArgs: (raw) => raw,
handler: async () => ({ granted: true }),
});
import { dispatch } from './dispatch.js';
import type { CallerContext } from './frame.js';
const AGENT_CTX: CallerContext = { caller: 'agent', sessionId: 's1', agentGroupId: 'g1', messagingGroupId: 'mg1' };
function grantRow(frameId: string, command: string): PendingApproval {
return {
approval_id: 'appr-123-abc',
session_id: 's1',
request_id: 'appr-123-abc',
action: 'cli_command',
payload: JSON.stringify({ frame: { id: frameId, command, args: {} }, callerContext: AGENT_CTX }),
created_at: new Date().toISOString(),
agent_group_id: 'g1',
channel_type: null,
platform_id: null,
platform_message_id: null,
expires_at: null,
status: 'pending',
title: 'CLI: groups-gated',
options_json: '[]',
approver_user_id: null,
approver_rule: 'admins-of-scope',
dedup_key: null,
};
}
function events(): Array<Record<string, any>> {
return appended.lines.map((l) => JSON.parse(l));
}
beforeEach(() => {
vi.clearAllMocks();
appended.lines.length = 0;
pendingRows.rows = [];
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
mockGetResource.mockImplementation((plural: string) => (plural === 'groups' ? { scopeField: 'id' } : undefined));
mockRequestApproval.mockResolvedValue(undefined);
});
describe('withAudit(dispatch)', () => {
it('records a success event for a host caller with socket origin and host actor', async () => {
const resp = await dispatch({ id: '1', command: 'groups-test', args: { foo: 'bar' } }, { caller: 'host' });
expect(resp.ok).toBe(true);
const [event] = events();
expect(event).toMatchObject({
schema_version: 1,
actor: { type: 'human', id: `host:${os.userInfo().username}`, email: null },
origin: { transport: 'socket' },
action: 'groups.test',
outcome: 'success',
correlation_id: null,
details: { args: ['foo'] },
});
// The value 'bar' is never stored — only the key name.
expect(event.details.foo).toBeUndefined();
});
it('records an agent command value-free, with container origin and channel', async () => {
await dispatch({ id: '1', command: 'groups-test', args: {} }, AGENT_CTX);
const [event] = events();
expect(event.actor).toMatchObject({ type: 'agent', id: 'g1' });
expect(event.origin).toEqual({
transport: 'container',
session_id: 's1',
messaging_group_id: 'mg1',
channel: 'slack',
});
expect(event.details).toEqual({ args: [] });
expect(event.resources).toContainEqual({ type: 'agent_group' });
});
it('records the passed flag names and echoes allowlisted safe values verbatim', async () => {
await dispatch(
{ id: '1', command: 'roles-grant', args: { role: 'admin', user: 'slack:U1' } },
{ caller: 'host' },
);
const [event] = events();
expect(event.action).toBe('roles.grant');
// Both flag names recorded; only the allowlisted `role` keeps its value.
expect(event.details.args).toEqual(['role', 'user']);
expect(event.details.role).toBe('admin');
expect(event.details.user).toBeUndefined();
// The user id still surfaces structurally, in resources.
expect(event.resources).toContainEqual({ type: 'user', id: 'slack:U1' });
});
it('records a denied event for a scope denial, naming the attempted resource type, with no free-text reason', async () => {
const resp = await dispatch({ id: '1', command: 'wirings-list', args: {} }, AGENT_CTX);
expect(resp.ok).toBe(false);
const [event] = events();
expect(event).toMatchObject({
action: 'wirings.list',
outcome: 'denied',
resources: [{ type: 'wirings' }],
details: { args: [], error: 'forbidden' },
});
// The denial message (which can echo caller input) is never stored.
expect(event.details.reason).toBeUndefined();
});
it('records a failure event with the error code only when the handler throws', async () => {
await dispatch({ id: '1', command: 'groups-fail', args: {} }, { caller: 'host' });
const [event] = events();
expect(event).toMatchObject({ action: 'groups.fail', outcome: 'failure', details: { error: 'handler-error' } });
expect(event.details.reason).toBeUndefined();
});
it('records a failure event and re-throws when the dispatcher itself throws', async () => {
mockRequestApproval.mockRejectedValueOnce(new Error('pending_approvals insert failed'));
await expect(dispatch({ id: '1', command: 'groups-gated', args: {} }, AGENT_CTX)).rejects.toThrow(
'pending_approvals insert failed',
);
const [event] = events();
expect(event).toMatchObject({ action: 'groups.gated', outcome: 'failure', details: { error: 'exception' } });
});
it('records a hold as a pending event correlated to the approval row it created', async () => {
pendingRows.rows = [grantRow('1', 'groups-gated')];
const resp = await dispatch({ id: '1', command: 'groups-gated', args: {} }, AGENT_CTX);
expect(resp.ok).toBe(false);
if (!resp.ok) expect(resp.error.code).toBe('approval-pending');
const [event] = events();
expect(event).toMatchObject({
action: 'groups.gated',
outcome: 'pending',
correlation_id: 'appr-123-abc',
});
expect(event.resources).toContainEqual({ type: 'approval', id: 'appr-123-abc' });
expect(event.details.error).toBeUndefined();
});
it('records an uncorrelated pending event when no approval row was created (no approver)', async () => {
pendingRows.rows = [];
await dispatch({ id: '1', command: 'groups-gated', args: {} }, AGENT_CTX);
const [event] = events();
expect(event).toMatchObject({ outcome: 'pending', correlation_id: null });
});
it('records an approved replay as a `success` event carrying the grant approval id', async () => {
// The approved/rejected verdict is the approvals.decide event's (approvals.audit.ts);
// the replayed command's terminal event is an ordinary success, chained by correlation_id.
const grant = grantRow('9', 'groups-gated');
mockGetPendingApproval.mockReturnValue(grant);
const resp = await dispatch({ id: '9', command: 'groups-gated', args: {} }, AGENT_CTX, { grant });
expect(resp.ok).toBe(true);
const [event] = events();
expect(event).toMatchObject({
action: 'groups.gated',
outcome: 'success',
correlation_id: 'appr-123-abc',
});
expect(event.resources).toContainEqual({ type: 'approval', id: 'appr-123-abc' });
});
it('records a --help probe under a neutral cli.help action, never the real verb', async () => {
const resp = await dispatch({ id: '1', command: 'groups-gated', args: { help: true } }, AGENT_CTX);
expect(resp.ok).toBe(true);
const [event] = events();
expect(event.action).toBe('cli.help');
expect(event.outcome).toBe('success');
expect(event.resources).toEqual([]);
});
it('records unknown commands as cli.unknown-command with the raw name in details', async () => {
await dispatch({ id: '1', command: 'nope-nothing', args: {} }, { caller: 'host' });
const [event] = events();
expect(event).toMatchObject({
action: 'cli.unknown-command',
outcome: 'failure',
resources: [],
details: { args: [], command: 'nope-nothing', error: 'unknown-command' },
});
});
it('records the resolved command and target id for dash-joined positional ids', async () => {
const uuid = '550e8400-e29b-41d4-a716-446655440000';
await dispatch({ id: '1', command: `groups-get-${uuid}`, args: {} }, { caller: 'host' });
const [event] = events();
expect(event).toMatchObject({ action: 'groups.get', outcome: 'success' });
expect(event.resources).toContainEqual({ type: 'agent_group', id: uuid });
// The id surfaces in resources; details keeps only the flag name.
expect(event.details.args).toContain('id');
expect(event.details.id).toBeUndefined();
});
it('normalizes hyphenated arg key names in details', async () => {
await dispatch({ id: '1', command: 'groups-test', args: { 'dry-run': 'true' } }, { caller: 'host' });
const [event] = events();
expect(event.details.args).toContain('dry_run');
expect(event.details.dry_run).toBeUndefined();
});
it('never stores arg values — a secret-bearing arg leaves only its key', async () => {
await dispatch(
{ id: '1', command: 'groups-test', args: { env: '{"NOTION_TOKEN":"tok-123","SAFE":"ok"}' } },
{ caller: 'host' },
);
const [event] = events();
expect(event.details.args).toContain('env');
expect(event.details.env).toBeUndefined();
// The secret never reaches the stored bytes.
expect(appended.lines[0]).not.toContain('NOTION_TOKEN');
expect(appended.lines[0]).not.toContain('tok-123');
});
});
@@ -1,264 +0,0 @@
/**
* CLI audit adapter (installed by /add-audit) — owns how the dispatcher
* describes itself to the audit log: the dispatch middleware plus the
* CLI-specific actor/origin/resource mapping. Composed in dispatch.ts as
* `export const dispatch = withAudit(dispatchInner)`; business logic there
* contains zero audit calls.
*
* Recording model: the log stores WHO did WHICH action to WHAT target and the
* outcome — never raw argument VALUES. `details` carries the arg key names
* that were passed plus a small allowlist of governance-relevant enum fields
* (role, mode, …) echoed verbatim; everything else is name-only. There is no
* value redactor because no free-form value is ever written — a secret can't
* leak from a field that was never stored. Target identifiers (ids, users,
* groups) are structured and surface separately in `resources`.
*
* Loading this module also boots the audit log (writability assert, boot
* prune, hook lifecycle, maintenance timer): dispatch.ts is imported by both
* transports during the host's barrel phase, so initAuditLog() runs before
* any command is accepted — and an enabled box with an unwritable
* data/audit/ refuses to start.
*/
import { emitAuditEvent } from '../audit/emit.js';
import { initAuditLog } from '../audit/init.js';
import {
type AuditActor,
type AuditEventInput,
type AuditOrigin,
type AuditOutcome,
type AuditResource,
} from '../audit/types.js';
import { containerOrigin, hostUser } from '../audit/vocab.js';
import { getPendingApprovalsByAction } from '../db/sessions.js';
import type { PendingApproval } from '../types.js';
import { getResource } from './crud.js';
import type { CallerContext, RequestFrame, ResponseFrame } from './frame.js';
import { commandGuardAction } from './guard.js';
import { type CommandDef, lookup } from './registry.js';
initAuditLog();
// ── CLI mapping ──
/**
* Host callers stamp `host:<install user>` daemon-side (the ncl socket is
* 0600 and owned by the install user); container callers are their agent group.
*/
export function actorForCaller(ctx: CallerContext): AuditActor {
return ctx.caller === 'host' ? { type: 'human', id: `host:${hostUser()}` } : { type: 'agent', id: ctx.agentGroupId };
}
export function originForCaller(ctx: CallerContext): AuditOrigin {
if (ctx.caller === 'host') return { transport: 'socket' };
return containerOrigin(ctx.sessionId, ctx.messagingGroupId || null);
}
/**
* Frame-level args use `--hyphen-keys`; recorded key names use the same
* underscore form the parsed handlers see. Mirrors crud's normalizeArgs
* (kept local so audit doesn't depend on a module tests commonly mock).
*/
export function normalizeArgKeys(raw: Record<string, unknown>): Record<string, unknown> {
const out: Record<string, unknown> = {};
for (const [k, v] of Object.entries(raw)) {
out[k.replace(/-/g, '_')] = v;
}
return out;
}
/**
* Arg values safe to record verbatim: governance-relevant enums that are never
* secrets and whose value IS the audit-worthy detail (which role was granted,
* which scope/mode was set). Every other value is dropped — only the presence
* of the flag (its key) is kept — so no free-form or secret-bearing value ever
* reaches disk.
*/
const SAFE_VALUE_FIELDS: ReadonlySet<string> = new Set([
'role',
'mode',
'session_mode',
'cli_scope',
'access',
'engage_mode',
'sender_scope',
'provider',
'model',
]);
/** CLI resource plural → audit resource type, where the singular isn't it. */
const RESOURCE_TYPE_OVERRIDES: Record<string, string> = {
groups: 'agent_group',
'messaging-groups': 'messaging_group',
'dropped-messages': 'dropped_message',
'user-dms': 'user_dm',
};
/**
* Derive touched/attempted resources from a command's args. Generic by design:
* `id` → the command's own resource, group/user args → their types, and a bare
* `{type}` entry when nothing else is known (a denied `users list` still names
* what was attempted). Ids are structured identifiers, not secrets.
*/
export function resourcesForCli(cmd: CommandDef, args: Record<string, unknown>): AuditResource[] {
if (!cmd.resource) return [];
const type = RESOURCE_TYPE_OVERRIDES[cmd.resource] ?? getResource(cmd.resource)?.name ?? cmd.resource;
const out: AuditResource[] = [];
const push = (t: string, id: unknown): void => {
if (typeof id !== 'string' || !id) return;
if (!out.some((r) => r.type === t && r.id === id)) out.push({ type: t, id });
};
push(type, args.id);
push('agent_group', args.agent_group_id ?? args.group);
push('user', args.user);
if (out.length === 0) out.push({ type });
return out;
}
// ── Command resolution, mirrored for the record ──
// Dispatch resolves the command on a local copy of the frame that never leaves
// it, so the middleware mirrors the one documented mechanic below. The mirror
// is mechanical, and drift only ever degrades a record's detail (a fallback
// action name) — never dispatch behavior, and never an outcome.
/**
* Mirror of dispatch's command resolution: exact lookup, then the longest
* registered dash-prefix with the remainder recorded as --id.
*/
function resolveForRecord(req: RequestFrame): { cmd?: CommandDef; args: Record<string, unknown> } {
const direct = lookup(req.command);
if (direct) return { cmd: direct, args: req.args };
let shortened = req.command;
let idx: number;
while ((idx = shortened.lastIndexOf('-')) > 0) {
shortened = shortened.slice(0, idx);
const fallback = lookup(shortened);
if (fallback) {
const tail = req.command.slice(shortened.length + 1);
return { cmd: fallback, args: { ...req.args, id: req.args.id ?? tail } };
}
}
return { args: req.args };
}
/**
* The approval row a hold just created for this frame — it gives the pending
* event the same correlation_id the approved replay will carry as its guard
* grant. requestApproval keeps the minted id internal, so the row is
* recovered by the frame id it stored in its payload; no row (e.g. no
* configured approver) → the hold is still recorded, uncorrelated.
*/
function holdApprovalIdFor(frameId: string): string | null {
const rows = getPendingApprovalsByAction('cli_command');
for (let i = rows.length - 1; i >= 0; i--) {
try {
const payload = JSON.parse(rows[i].payload) as { frame?: { id?: string } };
if (payload.frame?.id === frameId) return rows[i].approval_id;
// eslint-disable-next-line no-catch-all/no-catch-all -- a row with an unparseable payload is simply not this frame's hold
} catch {
continue;
}
}
return null;
}
// ── The dispatch middleware ──
type DispatchInner = (
req: RequestFrame,
ctx: CallerContext,
opts?: { grant?: PendingApproval },
) => Promise<ResponseFrame>;
/**
* Build the audit record for one dispatch. `res` is the response frame, or
* null when `inner` threw (`err` set) — a crash still leaves a record.
*
* Outcome: ok → success (an approved replay included — the approvals.decide
* event owns the approved/rejected verdict), forbidden → denied (captures
* pre-handler scope denials), approval-pending → pending (the record of a
* hold), a thrown/other error → failure. A `--help`
* probe is introspection, not the verb, so it records under a neutral
* `cli.help` action with no target — never as the real command succeeding.
* Correlation is the approval id: a replay carries the row as its grant, and a
* fresh hold recovers the row it just created.
*/
function buildEvent(
req: RequestFrame,
ctx: CallerContext,
opts: { grant?: PendingApproval },
res: ResponseFrame | null,
err: unknown,
): AuditEventInput {
const resolved = resolveForRecord(req);
const cmd = resolved.cmd;
const normArgs = normalizeArgKeys(resolved.args);
const isHelp = req.args.help === true && !!res && res.ok;
const pending = !!res && !res.ok && res.error.code === 'approval-pending';
// An approved replay records as ordinary `success`; the approvals.decide
// event (approvals.audit.ts) owns the approved/rejected verdict, chained to
// this terminal event by the same correlation_id (the grant's approval id).
const outcome: AuditOutcome = !res
? 'failure' // inner threw
: res.ok
? 'success'
: res.error.code === 'forbidden'
? 'denied'
: pending
? 'pending'
: 'failure';
// details: arg key names + allowlisted safe values only. No free-form value
// is ever stored, so nothing needs redacting and nothing can leak.
const details: Record<string, unknown> = { args: Object.keys(normArgs).sort() };
for (const f of SAFE_VALUE_FIELDS) {
if (normArgs[f] !== undefined) details[f] = normArgs[f];
}
if (err) {
details.error = 'exception'; // the throw's message is never stored
} else if (res && !res.ok && !pending) {
details.error = res.error.code; // the error CODE (a safe enum), never the free-text message
}
if (!cmd) details.command = req.command;
const correlationId = opts.grant?.approval_id ?? (pending ? holdApprovalIdFor(req.id) : null);
const action = isHelp ? 'cli.help' : cmd ? commandGuardAction(cmd) : 'cli.unknown-command';
const resources: AuditResource[] = isHelp ? [] : cmd ? resourcesForCli(cmd, normArgs) : [];
if (correlationId) resources.push({ type: 'approval', id: correlationId });
return {
actor: actorForCaller(ctx),
origin: originForCaller(ctx),
action,
resources,
outcome,
correlationId,
details,
};
}
/**
* Dispatch middleware — the exported `dispatch` is the wrapped function, so
* the socket server, the container delivery-action, and the in-module
* approved replay are all covered by the one composition.
*
* The emit brackets `inner` so a thrown dispatcher still leaves a record: on
* throw we emit a `failure` event and re-raise unchanged, so an approval-gated
* command whose hold crashes on the DB write is not a silent governance gap.
*/
export function withAudit(inner: DispatchInner): DispatchInner {
return async (req, ctx, opts = {}) => {
let res: ResponseFrame;
try {
res = await inner(req, ctx, opts);
} catch (err) {
emitAuditEvent(() => buildEvent(req, ctx, opts, null, err));
throw err;
}
emitAuditEvent(() => buildEvent(req, ctx, opts, res, null));
return res;
};
}
@@ -1,69 +0,0 @@
import { listAuditEvents } from '../../audit/reader.js';
import { registerResource } from '../crud.js';
/**
* Read-only audit resource (installed by /add-audit). The "table" is the
* NDJSON day-file store — no generic CRUD verbs are declared, so it is never
* queried as SQL; `list` is a custom operation backed by the audit reader.
*
* The filters are declared as the op's `args` (not `columns`), so crud
* validates them: a typo'd flag (`--actro`) is rejected instead of silently
* ignored, and `ncl audit list --help` lists every filter.
*
* Deliberately NOT on the group-scope allowlist (GROUP_SCOPE_RESOURCES):
* audit spans agent groups, so group-scoped agents are refused before the
* handler — the resource is host + `cli_scope: global` only, by omission
* (fails closed).
*/
registerResource({
name: 'audit_event',
plural: 'audit',
table: '(data/audit/*.ndjson)',
description: 'Local audit log — one event per ncl command. Newest first. Requires AUDIT_ENABLED=true.',
idColumn: 'event_id',
columns: [],
operations: {},
customOperations: {
list: {
access: 'open',
description: 'Query audit events, newest first. --format ndjson streams the stored lines for SIEM export.',
args: [
{
name: 'actor',
type: 'string',
description: 'Filter: exact actor id (host:<user>, <channel>:<handle>, agent group id)',
},
{ name: 'action', type: 'string', description: 'Filter: exact action or dotted prefix (e.g. groups.config)' },
{ name: 'resource', type: 'string', description: 'Filter: matches any event resource by id or type' },
{
name: 'outcome',
type: 'string',
description: 'Filter: event outcome',
enum: ['success', 'failure', 'denied', 'pending', 'approved'],
},
{ name: 'since', type: 'string', description: 'Window start: 7d / 24h / 30m relative, or ISO date/datetime (UTC)' },
{ name: 'until', type: 'string', description: 'Window end: same formats as --since' },
{ name: 'correlation', type: 'string', description: 'Filter: approval id tying a gated chain together' },
{
name: 'limit',
type: 'number',
description: 'Max events (default 100 for the table; ndjson exports every event in the window)',
},
{ name: 'format', type: 'string', description: 'Output format', enum: ['ndjson'] },
],
examples: [
'ncl audit list --outcome denied --since 7d',
'ncl audit list --correlation appr-1751970000000-x1y2z3',
'ncl audit list --action groups.config --format ndjson',
],
handler: async (args) => listAuditEvents(args),
formatHuman: (data) => {
// NDJSON export prints the stored lines verbatim; for the table view
// this throws so dispatch falls back to the row objects, which every
// client already renders.
if (typeof data === 'string') return data;
throw new Error('render rows client-side');
},
},
},
});
@@ -1,181 +0,0 @@
/**
* Approvals audit adapter — what the two lifecycle observers record. Audit is
* force-enabled and the store's append is captured; the observer registration
* is captured through a mock of the approvals primitive, then invoked directly.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { PendingApproval, Session } from '../../types.js';
const appended = vi.hoisted(() => ({ lines: [] as string[] }));
const handlers = vi.hoisted(
() =>
({ requested: undefined, resolved: undefined }) as {
requested?: (e: unknown) => unknown;
resolved?: (e: unknown) => unknown;
},
);
vi.mock('../../audit/config.js', () => ({ AUDIT_ENABLED: true, AUDIT_RETENTION_DAYS: 90 }));
vi.mock('../../audit/init.js', () => ({ initAuditLog: vi.fn(), maintainAudit: vi.fn() }));
vi.mock('../../audit/store.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../../audit/store.js')>();
return {
...actual,
appendAuditLine: (line: string) => {
appended.lines.push(line);
},
};
});
vi.mock('../../log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
// containerOrigin (via vocab) looks up the messaging group's channel.
vi.mock('../../db/messaging-groups.js', () => ({ getMessagingGroup: () => ({ channel_type: 'slack' }) }));
// Capture the observer handlers the adapter registers at module load.
vi.mock('./primitive.js', () => ({
registerApprovalRequestedHandler: (h: (e: unknown) => unknown) => {
handlers.requested = h;
},
registerApprovalResolvedHandler: (h: (e: unknown) => unknown) => {
handlers.resolved = h;
},
}));
// Loads the adapter (registers into `handlers`) with the mocks in place.
import './approvals.audit.js';
function events(): Array<Record<string, any>> {
return appended.lines.map((l) => JSON.parse(l));
}
function last(): Record<string, any> {
return events()[appended.lines.length - 1];
}
const AGENT_SESSION: Session = {
id: 's1',
agent_group_id: 'g1',
messaging_group_id: 'mg1',
thread_id: null,
agent_provider: null,
status: 'active',
container_status: 'running',
last_active: null,
created_at: '2026-01-01T00:00:00.000Z',
};
function hold(action: string, over: Partial<PendingApproval> = {}): PendingApproval {
return {
approval_id: 'appr-1',
session_id: 's1',
request_id: 'appr-1',
action,
payload: '{}',
created_at: '2026-01-01T00:00:00.000Z',
agent_group_id: 'g1',
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',
dedup_key: null,
...over,
};
}
beforeEach(() => {
appended.lines = [];
});
describe('approvals audit — request (pending)', () => {
it('records a non-CLI hold as pending: dotted action, picked approver, container origin', async () => {
await handlers.requested!({ approval: hold('add_mcp_server'), session: AGENT_SESSION, deliveredTo: 'slack:UADMIN' });
const e = last();
expect(e.action).toBe('self_mod.add_mcp_server');
expect(e.outcome).toBe('pending');
expect(e.actor).toEqual({ type: 'agent', id: 'g1', email: null, user_id: null, group_ids: null });
expect(e.origin).toEqual({ transport: 'container', session_id: 's1', messaging_group_id: 'mg1', channel: 'slack' });
expect(e.correlation_id).toBe('appr-1');
expect(e.resources).toContainEqual({ type: 'agent_group', id: 'g1' });
expect(e.resources).toContainEqual({ type: 'approval', id: 'appr-1' });
expect(e.resources).toContainEqual({ type: 'user', id: 'slack:UADMIN' });
});
it('skips cli_command — the dispatch adapter owns those holds', async () => {
await handlers.requested!({ approval: hold('cli_command'), session: AGENT_SESSION, deliveredTo: 'slack:UADMIN' });
expect(appended.lines).toHaveLength(0);
});
it('a sessionless sender hold records as the system actor on the host', async () => {
await handlers.requested!({
approval: hold('sender_admit', { agent_group_id: null, session_id: null }),
session: null,
deliveredTo: 'slack:UADMIN',
});
const e = last();
expect(e.action).toBe('senders.admit');
expect(e.actor.type).toBe('system');
expect(e.origin).toEqual({ transport: 'socket' });
// no agent_group resource when host-raised
expect(e.resources.some((r: any) => r.type === 'agent_group')).toBe(false);
});
});
describe('approvals audit — decision (approvals.decide)', () => {
it('an approve records approved; actor is the deciding admin on their channel', async () => {
await handlers.resolved!({ approval: hold('add_mcp_server'), session: AGENT_SESSION, outcome: 'approve', userId: 'slack:UADMIN' });
const e = last();
expect(e.action).toBe('approvals.decide');
expect(e.outcome).toBe('approved');
expect(e.actor).toEqual({ type: 'human', id: 'slack:UADMIN', email: null, user_id: null, group_ids: null });
expect(e.origin).toEqual({ transport: 'channel', channel: 'slack' });
expect(e.correlation_id).toBe('appr-1');
expect(e.details.gated_action).toBe('self_mod.add_mcp_server');
expect(e.details.requested_by).toBe('g1');
});
it('a reject records rejected, naming the gated action', async () => {
await handlers.resolved!({ approval: hold('a2a_message_gate'), session: AGENT_SESSION, outcome: 'reject', userId: 'discord:UMOD' });
const e = last();
expect(e.action).toBe('approvals.decide');
expect(e.outcome).toBe('rejected');
expect(e.origin.channel).toBe('discord');
expect(e.details.gated_action).toBe('a2a.send');
});
it('expiry decides as the system actor: rejected + reason expired', async () => {
await handlers.resolved!({ approval: hold('onecli_credential'), session: null, outcome: 'expire', userId: '' });
const e = last();
expect(e.actor).toEqual({ type: 'system', id: 'host', email: null, user_id: null, group_ids: null });
expect(e.origin).toEqual({ transport: 'socket' });
expect(e.outcome).toBe('rejected');
expect(e.details.reason).toBe('expired');
expect(e.details.gated_action).toBe('onecli.credential.use');
});
it('startup sweep decides as the system actor with reason swept', async () => {
await handlers.resolved!({ approval: hold('onecli_credential'), session: null, outcome: 'sweep', userId: '' });
const e = last();
expect(e.actor.type).toBe('system');
expect(e.outcome).toBe('rejected');
expect(e.details.reason).toBe('swept');
});
});
describe('approvals audit — correlation chain', () => {
it('the request and the decision share the approval id', async () => {
const approval = hold('create_agent', { approval_id: 'appr-xyz', request_id: 'appr-xyz' });
await handlers.requested!({ approval, session: AGENT_SESSION, deliveredTo: 'slack:UADMIN' });
await handlers.resolved!({ approval, session: AGENT_SESSION, outcome: 'approve', userId: 'slack:UADMIN' });
const [pending, decide] = events();
expect(pending.action).toBe('agents.create');
expect(pending.outcome).toBe('pending');
expect(pending.correlation_id).toBe('appr-xyz');
expect(decide.action).toBe('approvals.decide');
expect(decide.correlation_id).toBe('appr-xyz');
});
});
@@ -1,109 +0,0 @@
/**
* Approvals audit adapter (installed by /add-audit) — records the approval
* lifecycle for every host-routed hold, whichever stack created it: CLI
* commands, self-mod (install_packages / add_mcp_server), a2a create-agent and
* message-gate, unknown-sender admission, OneCLI credential holds, and channel
* registration. It subscribes to the two approval lifecycle observers, so the
* approval flows themselves contain zero audit calls.
*
* - request → a `pending` event naming the picked approver (a `user`
* resource), correlated by the approval id. `cli_command` is skipped: the
* CLI adapter (dispatch.audit.ts) already records those holds with the
* command's own dotted action + arg names — the mapping it alone owns.
* - resolve → an `approvals.decide` event (approved / rejected; expiry and
* sweep decide as the system actor), same correlation id. The gated action
* it decided rides `details.gated_action`.
*
* The approved/rejected verdict lives here; a CLI command's replayed execution
* records its own terminal success/failure through the dispatch adapter, on the
* same correlation id. Non-CLI holds have no separate execution event in this
* increment — the request and decision are the durable governance record, which
* is what survives the pending_approvals row being deleted on resolution.
*
* Recording model matches the rest of the log: WHO decided WHAT, never raw
* payload values — the gated action name and the approval id, not its args.
*/
import { emitAuditEvent } from '../../audit/emit.js';
import { initAuditLog } from '../../audit/init.js';
import type { AuditActor, AuditOrigin, AuditOutcome, AuditResource } from '../../audit/types.js';
import { approvalActionName, channelOf, channelOrigin, containerOrigin } from '../../audit/vocab.js';
import type { PendingApproval, Session } from '../../types.js';
import {
type ApprovalRequestedEvent,
type ApprovalResolvedEvent,
registerApprovalRequestedHandler,
registerApprovalResolvedHandler,
} from './primitive.js';
// Loading this adapter also boots the audit log when enabled (idempotent — the
// dispatch adapter may have run it already), so the approval surface records
// even on a build that composed it before the CLI middleware.
initAuditLog();
/** The agent group whose action raised the hold, or null for host-raised holds. */
function requesterOf(approval: PendingApproval, session: Session | null): string | null {
return approval.agent_group_id ?? session?.agent_group_id ?? null;
}
/** The requester as an actor: the agent group, else the host (system-raised). */
function requesterActor(agentGroupId: string | null): AuditActor {
return agentGroupId ? { type: 'agent', id: agentGroupId } : { type: 'system', id: 'host' };
}
/** Origin of the requesting side: the container session if any, else the host socket. */
function requesterOrigin(session: Session | null): AuditOrigin {
return session ? containerOrigin(session.id, session.messaging_group_id) : { transport: 'socket' };
}
// request → pending. cli_command holds belong to the dispatch adapter (correct
// dotted action + args); every other gated surface is recorded here.
registerApprovalRequestedHandler((event: ApprovalRequestedEvent) => {
if (event.approval.action === 'cli_command') return;
emitAuditEvent(() => {
const agentGroupId = requesterOf(event.approval, event.session);
const resources: AuditResource[] = [];
if (agentGroupId) resources.push({ type: 'agent_group', id: agentGroupId });
resources.push({ type: 'approval', id: event.approval.approval_id });
if (event.deliveredTo) resources.push({ type: 'user', id: event.deliveredTo });
return {
actor: requesterActor(agentGroupId),
origin: requesterOrigin(event.session),
action: approvalActionName(event.approval.action),
resources,
outcome: 'pending' as AuditOutcome,
correlationId: event.approval.approval_id,
};
});
});
const RESOLVED_OUTCOME: Record<ApprovalResolvedEvent['outcome'], AuditOutcome> = {
approve: 'approved',
reject: 'rejected',
expire: 'rejected',
sweep: 'rejected',
};
// resolve → approvals.decide. A human clicker is the actor (answered on a chat
// platform); expiry and startup sweep decide as the system actor.
registerApprovalResolvedHandler((event: ApprovalResolvedEvent) => {
emitAuditEvent(() => {
const agentGroupId = requesterOf(event.approval, event.session);
const bySystem = !event.userId || event.outcome === 'expire' || event.outcome === 'sweep';
const resources: AuditResource[] = [];
if (agentGroupId) resources.push({ type: 'agent_group', id: agentGroupId });
resources.push({ type: 'approval', id: event.approval.approval_id });
const details: Record<string, unknown> = { gated_action: approvalActionName(event.approval.action) };
if (agentGroupId) details.requested_by = agentGroupId;
if (event.outcome === 'expire') details.reason = 'expired';
if (event.outcome === 'sweep') details.reason = 'swept';
return {
actor: bySystem ? { type: 'system', id: 'host' } : { type: 'human', id: event.userId },
origin: bySystem ? { transport: 'socket' } : channelOrigin(channelOf(event.userId)),
action: 'approvals.decide',
resources,
outcome: RESOLVED_OUTCOME[event.outcome],
correlationId: event.approval.approval_id,
details,
};
});
});
+31 -5
View File
@@ -7,6 +7,34 @@ description: Add WhatsApp channel via native Baileys adapter. Direct connection
Adds WhatsApp support via the native Baileys adapter (no Chat SDK bridge).
## Number safety check (required)
Complete this check before running any install or authentication command. If the user already said they want to use their **shared**, **personal**, **main**, **existing**, or **everyday** WhatsApp number, treat it as a shared number and show the warning immediately. Do not ask the number-type question again.
Otherwise, use `AskUserQuestion`:
**Which WhatsApp number will NanoClaw use?**
- **Dedicated number (Recommended)** — a separate number used only for NanoClaw
- **Shared / personal number** — the user's existing everyday WhatsApp number
Remember the answer as `NUMBER_MODE` for the rest of this workflow.
If `NUMBER_MODE=shared`, show this warning exactly:
> ⚠️ **Risk to your WhatsApp account**
>
> Connecting your shared or personal number could cause WhatsApp to temporarily suspend or permanently ban that number. You could lose access to the WhatsApp account, chats, and groups you rely on.
>
> We strongly recommend using a separate, dedicated number for NanoClaw.
Then use `AskUserQuestion`:
- **Go back and use a dedicated number (Recommended)**
- **I understand the risk — continue with my shared number**
Do not continue with installation or authentication unless the user explicitly selects the second option. If they choose a dedicated number, set `NUMBER_MODE=dedicated` and continue without showing the warning again.
## Install
NanoClaw doesn't ship channels in trunk. This skill copies the native WhatsApp (Baileys) adapter and its `whatsapp-auth` setup step in from the `channels` branch. No Chat SDK bridge.
@@ -92,7 +120,7 @@ WhatsApp uses linked-device authentication — no API key, just a one-time pairi
### Check current state
Check if WhatsApp is already authenticated. If `store/auth/creds.json` exists, skip to "Dedicated vs personal number".
Check if WhatsApp is already authenticated. The number safety check above is still required even when credentials already exist. If `store/auth/creds.json` exists, skip to "Dedicated vs personal number" after completing the safety check.
```bash
test -f store/auth/creds.json && echo "WhatsApp auth exists" || echo "No WhatsApp auth"
@@ -209,9 +237,7 @@ The adapter behaves fundamentally differently depending on whether the linked nu
- **Shared/personal number** (`ASSISTANT_HAS_OWN_NUMBER` unset or not `true`) — DMs to this number and group @-tags of it address the *human*, not the bot. The adapter never emits a mention signal (`mentions: 'never'` in its declared channel defaults), so: no stranger DM ever auto-creates a messaging group or raises an admin approval card; group wirings default to a name pattern (`\b<AgentName>\b`) instead of platform mentions; auto-created chats default to `unknown_sender_policy: 'strict'`; outbound messages are prefixed with the assistant's name.
- **Dedicated number** (`ASSISTANT_HAS_OWN_NUMBER=true`) — everything sent to the number is for the bot. DMs and group mentions carry a real mention signal (`mentions: 'platform'`), unknown senders escalate via `request_approval` approval cards, and card-approved groups wire with `engage_mode: 'mention'`. No name prefix on outbound.
AskUserQuestion: Is this a shared phone number (personal WhatsApp) or a dedicated number?
- **Shared number** — your personal WhatsApp (bot prefixes messages with its name)
- **Dedicated number** — a separate phone/SIM for the assistant
Use the `NUMBER_MODE` selected in the required safety check. If information discovered later contradicts that selection, ask again before changing modes; switching to shared requires the same warning and explicit acknowledgement.
Write the answer to `.env` **explicitly in both cases** (don't rely on the inference rule for new installs):
@@ -224,7 +250,7 @@ ASSISTANT_HAS_OWN_NUMBER=false
### Update path: existing install, flag unset
If WhatsApp auth already exists (`store/auth/creds.json` present) but `.env` has no `ASSISTANT_HAS_OWN_NUMBER` line, the install predates the explicit switch. Ask the operator which mode applies and write it explicitly.
If WhatsApp auth already exists (`store/auth/creds.json` present) but `.env` has no `ASSISTANT_HAS_OWN_NUMBER` line, the install predates the explicit switch. Use the mode established by the required safety check and write it explicitly.
Suggest a default by comparing the authed number against the wired DM chat:
+2 -4
View File
@@ -4,13 +4,11 @@ All notable changes to NanoClaw will be documented in this file.
## [Unreleased]
- **New skill: `/add-audit` — opt-in local audit log for the `ncl` surface.** Installs an append-only, SIEM-shaped audit log: every command through the dispatcher (both transports — host socket and container — including scope denials, approval holds, and approved replays) becomes one canonical NDJSON event under `data/audit/<UTC-day>.ndjson`. Gated chains share a `correlation_id` (the approval id: the hold's `pending` event and the replay's terminal event carry the same one); `details` record only the flag names passed plus an allowlist of safe enum values (`role`, `mode`, …), so raw argument values — where secrets hide — are never written. `AUDIT_RETENTION_DAYS` (default 90; `0` = keep forever) hard-deletes day-files past the horizon at boot and once per UTC day. Read back with `ncl audit list [--actor --action --resource --outcome --since --until --correlation --limit] [--format ndjson]` — host + global-scope callers only (the resource stays off the group-scope allowlist, so group-scoped agents fail closed). In-process exporters plug in via `registerAuditHook` — post-write hooks that fire only after the local append succeeds. Off by default: nothing is persisted (and `data/audit/` is never created) until `AUDIT_ENABLED=true`, an enabled box refuses to boot if the directory isn't writable, and a disabled box answers `ncl audit list` with a clear error instead of an empty list. The skill's whole core footprint is the dispatch composition (`export const dispatch = withAudit(dispatchInner)`) plus the resource-barrel import, both guarded by a shipped wiring test. Approval-lifecycle events (request/decision as their own events), OneCLI credential holds, and channel/sender events are later increments on the same schema.
- **The guard seam.** 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 flow's existing approval mechanics), or `deny`. The keyed registries (ncl commands, delivery actions) wrap their handlers at registration, so the guarded path is the only path by construction: every ncl command derives its guard from its own definition inside `register()`, and a delivery action registers with a guard spec or an explicit `unguarded(<reason>)` declaration — omission doesn't compile, and `grep "unguarded("` is the complete justified inventory. The broadcast hooks (response handlers, message interceptors) stay plain registrations; the one privileged click path — channel registration — consults the guard inline in its handler, like the a2a route and the unknown-sender gate. Consults carry the branded `GuardedAction` value returned by `defineGuardedAction`, so a typo'd or dropped wiring is a compile error and a forged value is denied at runtime — never a fail-open. The decision 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, channel-registration click auth, host-as-trusted-caller in code). Policy-as-data (tighten-only rule sources composing with the decision) is deliberately deferred — a generalized rules table can arrive later, 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 checks on every replay. Two outcome changes are inherent to that replay semantics and ship deliberately: **(a)** approving a held a2a message after its destination was revoked no longer delivers it (the checks re-run live); **(b)** a forged, already-consumed, or wrong-action/wrong-payload grant refuses the replay instead of executing (previously any in-process caller passing `approved: true` executed). Zero rules ship by default — the seeded posture is today's behavior. (The D1/D2/D4 click-auth fixes from the guarded-actions defect inventory are NOT in this change — they belong to the approval-contract PR.)
- **Delivery-registry 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. (The cross-registry pairing — every holding action has a registered approve continuation — is enforced by the conformance test; at runtime a missing continuation already resolves loudly, telling the requester no handler is installed.)
- **The guard seam.** Every privileged action crossing the container or channel boundary now passes one decision function — `guard()` in `src/guard/` — before it executes: `allow`, `hold` (the existing approval flows), or `deny`. Today's checks are preserved verbatim as each action's decision; ncl commands and delivery actions cannot register without a guard, and approved replays re-enter carrying the approval row as a **grant** (the forgeable `approved: true` boolean is deleted) with the checks re-run live. Two deliberate outcome changes: **(a)** approving a held a2a message after its destination was revoked no longer delivers it — the requester is told "approved, but not delivered" and the host logs a warning; **(b)** a forged, already-consumed, or mismatched grant refuses the replay instead of executing.
- **Delivery-registry hardening.** Re-registering a guard-wrapped delivery action *without* a guard spec now throws instead of silently disarming the guard.
- [BREAKING] **`whatsapp-formatting` and `slack-formatting` container skills moved from trunk to the `channels` branch.** They now install with their channel — `/add-whatsapp` / `/add-slack` and the setup installers copy them in — so installs without those channels stop carrying channel-specific formatting instructions in every agent's context. **Migration — only if this install has the channel wired** (check `src/channels/whatsapp.ts` / `src/channels/slack.ts`): updating removes both skills from the working tree — WhatsApp agents lose the formatting fragment from their composed CLAUDE.md on next spawn, Slack agents lose the mrkdwn skill from `~/.claude/skills`. Re-run the matching skill, `/add-whatsapp` or `/add-slack` (idempotent), to restore. Installs without the channel need nothing — do NOT run the add-skill just in case; it installs the full channel adapter.
- **Pre-task script failures back their series off instead of spinning.** A `--script` that errors lands the occurrence as a failed run (`script-skip:error` ack → `failed` status); recurrence reads the series' trailing failed streak and re-arms at `max(cron next, now + 2·2^(n1) min, cap 60)`; after 8 consecutive failures the series is auto-paused with a host-written note in its run log (`ncl tasks resume` revives it). A deliberate `wakeAgent:false` gate is a normal run and never backs off. Also fixed: an explicitly-addressed `<message to>` in a task fire's final text now delivers as a deliberate send (previously suppressed as a turn-reply echo → zero delivery when the agent skipped the MCP tool); identical echoes of an MCP send are dropped in the runner, where the duplication originates.
- [BREAKING] **Scheduled tasks moved from MCP tools to `ncl tasks`.** The six scheduling MCP tools are no longer exposed to agent containers; agents and operators manage tasks with `ncl tasks list/get/create/update/cancel/pause/resume/delete`. New tasks run from a per-agent-group system session rather than waking the chat session that created them, and task writes are not approval-gated inside the owning group. **Migration:** [docs/ncl-tasks-migration.md](docs/ncl-tasks-migration.md).
- **One approval contract across every hold.** All approval holds now share one hold-record shape on `pending_approvals` (approver rule, dedup key — migration 020, with backfills) 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, its `UNIQUE(messaging_group_id, sender_identity)` preserved as a partial UNIQUE on the hold's dedup key; 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 rule. Observers now see the full hold lifecycle with zero touch points inside the flows: `registerApprovalRequestedHandler` (new, the creation-side sibling of `registerApprovalResolvedHandler`) fires once whenever a hold record comes into existence, whichever stack created it — `requestApproval`, the OneCLI credential bridge, channel registration (as a synthesized hold view) — and every resolution announces through the approval-resolved observer (outcome `approve` | `reject` | `expire` | `sweep`, session nullable). This is a behavior-preserving refactor: unifying the three flows changes no click-authorization decision versus today.
- **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 -4
View File
@@ -62,15 +62,14 @@ For ad-hoc queries from skills or scripts, use the in-tree wrapper rather than t
| `src/index.ts` | Entry point: init DB, migrations, channel adapters, delivery polls, sweep, shutdown |
| `src/router.ts` | Inbound routing: messaging group → agent group → session → `inbound.db` → wake |
| `src/delivery.ts` | Polls `outbound.db`, delivers via adapter, handles system actions (schedule, approvals, etc.) |
| `src/delivery-guard.ts` | `DeliveryGuardSpec` + `runGuarded` — the precheck → guard → deny/hold/allow consult path for privileged delivery actions; the registry itself stays in `delivery.ts` |
| `src/delivery-guard.ts` | `DeliveryGuardSpec` + `runGuarded` — the guard-consult pipeline for privileged delivery actions (registry stays in `delivery.ts`) |
| `src/host-sweep.ts` | 60s sweep: `processing_ack` sync, stale detection, due-message wake, recurrence |
| `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` | Docker CLI wrapper (runtime binary, host-gateway args, mount args), orphan cleanup |
| `src/guard/` | Privileged-action decision seam: `guard(action, input)` → allow \| hold \| deny. Domain-free leaf; domain decisions are `defineGuardedAction` values exported from module-edge `guard.ts` adapters (cli, agent-to-agent, self-mod, permissions) — consults hold the value, so a missing/typo'd wiring is a compile error, never a fail-open. The keyed registries (ncl commands, delivery actions) demand a guard at registration (derived spec or explicit `unguarded(<reason>)`); broadcast hooks stay plain registrations, and the privileged click path — the channel-registration card handler — consults the guard inline, like the a2a route and the unknown-sender gate (the free-text name reply is deliberately not re-authorized; the click is the auth, as on main). Approved replays carry the approval row as a grant and re-run the structural checks. Policy-as-data (a runtime rules table) is deliberately deferred. Conformance test: `src/guard/conformance.test.ts` |
| `src/guard/` | Privileged-action decision seam: `guard(action, input)` → allow \| hold \| deny. Module-edge `guard.ts` adapters (cli, agent-to-agent, self-mod, permissions) define each action's decision; ncl commands + delivery actions demand a guard at registration; approved replays carry the approval row as a grant and re-run the checks. 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, 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`) for every hold |
| `src/modules/approvals/primitive.ts` | `pickApprover`, `pickApprovalDelivery`, `requestApproval`, approval-handler registry |
| `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 |
@@ -24,6 +24,10 @@ function log(msg: string): void {
// the question and blocks on the real reply.
// - EnterPlanMode / ExitPlanMode / EnterWorktree / ExitWorktree: Claude
// Code UI affordances; in a headless container they'd appear stuck.
// - DesignSync: desktop design-tool integration — nothing to sync with in a
// headless container (~9.3KB/turn schema).
// - ReportFindings: code-review-reporting UI affordance with no headless
// host surface to receive it (~1.9KB/turn schema).
const SDK_DISALLOWED_TOOLS = [
'CronCreate',
'CronDelete',
@@ -34,6 +38,8 @@ const SDK_DISALLOWED_TOOLS = [
'ExitPlanMode',
'EnterWorktree',
'ExitWorktree',
'DesignSync',
'ReportFindings',
];
// Tool allowlist for NanoClaw agent containers. MCP-tool entries are derived
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "nanoclaw",
"version": "2.1.46",
"version": "2.1.47",
"description": "Personal Claude assistant. Lightweight, secure, customizable.",
"type": "module",
"packageManager": "pnpm@10.33.0",
+4 -4
View File
@@ -1,5 +1,5 @@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="90" height="20" role="img" aria-label="232k tokens, 116% of context window">
<title>232k tokens, 116% of context window</title>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="90" height="20" role="img" aria-label="240k tokens, 120% of context window">
<title>240k tokens, 120% of context window</title>
<linearGradient id="s" x2="0" y2="100%">
<stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
<stop offset="1" stop-opacity=".1"/>
@@ -15,8 +15,8 @@
<g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" font-size="11">
<text aria-hidden="true" x="26" y="15" fill="#010101" fill-opacity=".3">tokens</text>
<text x="26" y="14">tokens</text>
<text aria-hidden="true" x="71" y="15" fill="#010101" fill-opacity=".3">232k</text>
<text x="71" y="14">232k</text>
<text aria-hidden="true" x="71" y="15" fill="#010101" fill-opacity=".3">240k</text>
<text x="71" y="14">240k</text>
</g>
</g>
</a>

Before

Width:  |  Height:  |  Size: 1.1 KiB

After

Width:  |  Height:  |  Size: 1.1 KiB

+63
View File
@@ -0,0 +1,63 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
const mocks = vi.hoisted(() => ({
brightSelect: vi.fn(),
note: vi.fn(),
userInput: vi.fn(),
}));
vi.mock('../lib/bright-select.js', () => ({
brightSelect: mocks.brightSelect,
}));
vi.mock('../lib/theme.js', async (importOriginal) => ({
...(await importOriginal<typeof import('../lib/theme.js')>()),
note: mocks.note,
}));
vi.mock('../logs.js', async (importOriginal) => ({
...(await importOriginal<typeof import('../logs.js')>()),
userInput: mocks.userInput,
}));
import { BACK_TO_CHANNEL_SELECTION } from '../lib/back-nav.js';
import { runWhatsAppChannel } from './whatsapp.js';
describe('WhatsApp shared-number risk gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('shows the warning and requires explicit acknowledgement for a shared number', async () => {
mocks.brightSelect.mockResolvedValueOnce('shared').mockResolvedValueOnce('continue').mockResolvedValueOnce('back');
const result = await runWhatsAppChannel('Daniel');
expect(result).toBe(BACK_TO_CHANNEL_SELECTION);
expect(mocks.note).toHaveBeenCalledWith(
expect.stringContaining('temporarily suspend or permanently ban that number'),
'Risk to your WhatsApp account',
);
expect(mocks.userInput).toHaveBeenCalledWith('whatsapp_shared_risk_acknowledged', 'true');
});
it('does not show the warning for a dedicated number', async () => {
mocks.brightSelect.mockResolvedValueOnce('dedicated').mockResolvedValueOnce('back');
const result = await runWhatsAppChannel('Daniel');
expect(result).toBe(BACK_TO_CHANNEL_SELECTION);
expect(mocks.note).not.toHaveBeenCalled();
expect(mocks.userInput).not.toHaveBeenCalledWith('whatsapp_shared_risk_acknowledged', expect.anything());
});
it('switches to dedicated mode when the user declines the shared-number risk', async () => {
mocks.brightSelect.mockResolvedValueOnce('shared').mockResolvedValueOnce('dedicated').mockResolvedValueOnce('back');
const result = await runWhatsAppChannel('Daniel');
expect(result).toBe(BACK_TO_CHANNEL_SELECTION);
expect(mocks.note).toHaveBeenCalledOnce();
expect(mocks.userInput).not.toHaveBeenCalledWith('whatsapp_shared_risk_acknowledged', expect.anything());
});
});
+29 -13
View File
@@ -6,8 +6,8 @@
*
* 1. Ask whether the agent gets a dedicated number or shares the
* operator's personal one. Personal interception screen that spells
* out self-chat-only mode and steers toward alternatives (default is
* back to channel selection)
* out the account-ban risk and self-chat-only mode (default is switching
* to a dedicated number)
* 2. Ask how to authenticate (QR code in terminal, default, or pairing code)
* 3. If pairing-code: collect the phone number
* 4. Install the adapter + Baileys + QR + pino via setup/add-whatsapp.sh
@@ -68,7 +68,7 @@ export async function runWhatsAppChannel(displayName: string): Promise<ChannelFl
let mode: 'dedicated' | 'shared' = ownership;
if (mode === 'shared') {
const proceed = await confirmSharedNumber();
if (proceed === 'back') return BACK_TO_CHANNEL_SELECTION;
if (proceed === 'dedicated') mode = 'dedicated';
}
const method = await askAuthMethod();
@@ -121,7 +121,7 @@ export async function runWhatsAppChannel(displayName: string): Promise<ChannelFl
// Chatting from the bot's own number IS the shared-number setup —
// route through the same interception screen as the up-front pick.
const proceed = await confirmSharedNumber();
if (proceed === 'back') return BACK_TO_CHANNEL_SELECTION;
if (proceed === 'dedicated') return BACK_TO_CHANNEL_SELECTION;
mode = 'shared';
}
}
@@ -221,13 +221,19 @@ async function askNumberOwnership(): Promise<'dedicated' | 'shared' | 'back'> {
}
/**
* Interception screen for the shared-number path: make the self-chat-only
* tradeoff explicit and steer toward alternatives before any install or
* auth happens. Default is bailing back to channel selection.
* Interception screen for the shared-number path: make the account-ban risk
* and self-chat-only tradeoff explicit before any install or auth happens.
* Default is switching to a dedicated number.
*/
async function confirmSharedNumber(): Promise<'continue' | 'back'> {
async function confirmSharedNumber(): Promise<'continue' | 'dedicated'> {
note(
[
'Connecting your shared or personal number could cause WhatsApp to',
'temporarily suspend or permanently ban that number. You could lose access',
'to the WhatsApp account, chats, and groups you rely on.',
'',
'We strongly recommend using a separate, dedicated number for NanoClaw.',
'',
'On your personal number, the agent lives only in your "You" / self-chat.',
'Messages other people send you are ignored entirely — never read, never',
'answered, never flagged for approval. Nobody else can talk to the agent.',
@@ -238,19 +244,29 @@ async function confirmSharedNumber(): Promise<'continue' | 'back'> {
`${brandBold('a dedicated WhatsApp number')} — spare SIM, eSIM, or old phone`,
`${brandBold('/add-whatsapp-cloud')} — the official Meta Business API`,
].join('\n'),
'Personal number = self-chat only',
'Risk to your WhatsApp account',
);
const choice = ensureAnswer(
await brightSelect({
message: 'How would you like to proceed?',
options: [
{ value: 'back', label: '← Pick a different channel' },
{ value: 'continue', label: 'Continue — self-chat only' },
{
value: 'dedicated',
label: 'Go back and use a dedicated number',
hint: 'recommended',
},
{
value: 'continue',
label: 'I understand the risk — continue with my shared number',
},
],
initialValue: 'back',
initialValue: 'dedicated',
}),
) as 'continue' | 'back';
) as 'continue' | 'dedicated';
setupLog.userInput('whatsapp_shared_confirm', choice);
if (choice === 'continue') {
setupLog.userInput('whatsapp_shared_risk_acknowledged', 'true');
}
return choice;
}
-27
View File
@@ -150,33 +150,6 @@ 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',
-12
View File
@@ -48,18 +48,6 @@ 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: 'dedup_key', type: 'string', description: 'In-flight dedup key (e.g. sender admission per chat+sender).' },
],
operations: { list: 'open', get: 'open' },
});
+6 -7
View File
@@ -109,13 +109,10 @@ 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_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);
`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());
db.prepare(
`INSERT INTO pending_channel_approvals (messaging_group_id, agent_group_id, original_message, approver_user_id, created_at)
@@ -151,9 +148,10 @@ describe('groups CLI delete cascades dependent rows (#2525)', () => {
expect(data.removed).toMatchObject({
sessions: 1,
pending_questions: 1,
pending_approvals: 2,
pending_approvals: 1,
agent_destinations_owned: 1,
agent_destinations_pointing: 0,
pending_sender_approvals: 1,
pending_channel_approvals: 1,
messaging_group_agents: 1,
agent_group_members: 1,
@@ -169,6 +167,7 @@ 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
@@ -134,6 +134,7 @@ 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,
@@ -162,6 +163,9 @@ 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;
@@ -1,99 +0,0 @@
/**
* Upgrade-path test for migration 020 (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 020 — 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, agent_group_id FROM pending_approvals').all() as Array<{
approval_id: string;
approver_rule: string;
agent_group_id: string;
}>;
const byId = Object.fromEntries(rows.map((r) => [r.approval_id, r]));
expect(byId['appr-a2a']).toMatchObject({
approver_rule: 'exclusive',
agent_group_id: 'ag-1',
});
expect(byId['appr-cli']).toMatchObject({
approver_rule: 'admins-of-scope',
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);
});
it('the dedup_key partial UNIQUE keeps at most one in-flight hold per key (sender fold parity)', () => {
runMigrations(db);
const now = new Date().toISOString();
const insert = (id: string, key: string | null) =>
db
.prepare(
`INSERT OR IGNORE INTO pending_approvals
(approval_id, request_id, action, payload, created_at, title, options_json, dedup_key)
VALUES (?, ?, 'sender_admit', '{}', ?, '', '[]', ?)`,
)
.run(id, id, now, key).changes;
expect(insert('a', 'sender_admit:mg:tg:stranger')).toBe(1);
expect(insert('b', 'sender_admit:mg:tg:stranger')).toBe(0); // same key → dropped by the partial UNIQUE
// NULL keys are exempt (the partial index), so ordinary holds still coexist.
expect(insert('c', null)).toBe(1);
expect(insert('d', null)).toBe(1);
});
});
@@ -1,48 +0,0 @@
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).
* - `dedup_key` in-flight dedup: while a pending row carries a key, a
* second request with the same key is dropped. The partial UNIQUE index
* enforces it at the DB level under concurrency, preserving the sender
* table's old UNIQUE(messaging_group_id, sender_identity) guarantee.
*
* 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 migration020: Migration = {
version: 20,
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 dedup_key TEXT;`);
// Partial UNIQUE preserves the dropped sender table's
// UNIQUE(messaging_group_id, sender_identity): at most one live hold per
// dedup key, so a concurrent duplicate request can't mint a second card.
db.exec(
`CREATE UNIQUE INDEX IF NOT EXISTS idx_pending_approvals_dedup
ON pending_approvals(dedup_key) WHERE dedup_key IS NOT NULL;`,
);
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
@@ -18,7 +18,6 @@ import { moduleApprovalsPendingApprovals } from './module-approvals-pending-appr
import { moduleApprovalsTitleOptions } from './module-approvals-title-options.js';
import { migration018 } from './018-approvals-approver-user-id.js';
import { migration019 } from './019-wiring-threads.js';
import { migration020 } from './020-holds-approver-rule.js';
export interface Migration {
version: number;
@@ -53,7 +52,6 @@ export const migrations: Migration[] = [
migration015,
migration016,
migration019,
migration020,
];
/** Row shape of PRAGMA foreign_key_check. Child rowids are stable across a
+15
View File
@@ -136,6 +136,21 @@ 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)
);
`;
/**
+11 -14
View File
@@ -202,11 +202,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, approver_rule, dedup_key)
title, options_json, approver_user_id)
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, @approver_rule, @dedup_key)`,
@title, @options_json, @approver_user_id)`,
)
.run({
session_id: null,
@@ -217,20 +217,11 @@ export function createPendingApproval(
expires_at: null,
status: 'pending',
approver_user_id: null,
approver_rule: 'admins-of-scope',
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
@@ -286,9 +277,8 @@ export function getAskQuestionRender(
| undefined;
if (a?.title) return { title: a.title, options: JSON.parse(a.options_json) };
// 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.)
// Channel-registration + unknown-sender approvals persist title/options_json
// the same way pending_approvals does — just SELECT and return.
if (hasTable(getDb(), 'pending_channel_approvals')) {
const c = getDb()
.prepare('SELECT title, options_json FROM pending_channel_approvals WHERE messaging_group_id = ?')
@@ -296,5 +286,12 @@ 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;
}
+1 -5
View File
@@ -459,11 +459,7 @@ export function registerDeliveryAction(
* 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('Approved replay arrived without a session — dropping', { action });
return;
}
return async (ctx: { session: Session; payload: Record<string, unknown>; approval: PendingApproval }) => {
const entry = deliveryActions.get(action);
if (!entry || isUnguardedEntry(entry)) {
log.warn('Approved replay for an action that is not guard-wrapped — dropping', { action });
+68
View File
@@ -0,0 +1,68 @@
import fs from 'fs';
import path from 'path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
const TEST_ROOT = '/tmp/nanoclaw-group-init-settings-test';
vi.mock('./config.js', async (importOriginal) => ({
...(await importOriginal<typeof import('./config.js')>()),
DATA_DIR: '/tmp/nanoclaw-group-init-settings-test/data',
GROUPS_DIR: '/tmp/nanoclaw-group-init-settings-test/groups',
}));
vi.mock('./log.js', () => ({
log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() },
}));
import { closeDb, createAgentGroup, initTestDb, runMigrations } from './db/index.js';
import { initGroupFilesystem } from './group-init.js';
import type { AgentGroup } from './types.js';
function makeGroup(id: string): AgentGroup {
const ag = { id, name: id, folder: id, agent_provider: null, created_at: new Date().toISOString() } as AgentGroup;
createAgentGroup(ag);
return ag;
}
beforeEach(() => {
fs.rmSync(TEST_ROOT, { recursive: true, force: true });
fs.mkdirSync(TEST_ROOT, { recursive: true });
runMigrations(initTestDb());
});
afterEach(() => {
closeDb();
fs.rmSync(TEST_ROOT, { recursive: true, force: true });
});
describe('default settings.json for new groups', () => {
it('is lean: no agent-teams env key, unmanaged keys intact', () => {
const ag = makeGroup('ag-lean');
initGroupFilesystem(ag, {});
const file = path.join(TEST_ROOT, 'data', 'v2-sessions', ag.id, '.claude-shared', 'settings.json');
const settings = JSON.parse(fs.readFileSync(file, 'utf-8'));
expect(settings.env.CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS).toBeUndefined();
expect(settings.env.CLAUDE_CODE_ADDITIONAL_DIRECTORIES_CLAUDE_MD).toBe('1');
expect(JSON.stringify(settings.hooks.PreCompact)).toContain('compact-instructions');
});
it('never rewrites an existing settings.json — a hand-edited re-enable sticks', () => {
const ag = makeGroup('ag-reenable');
initGroupFilesystem(ag, {});
const file = path.join(TEST_ROOT, 'data', 'v2-sessions', ag.id, '.claude-shared', 'settings.json');
// Operator re-enables both features by editing the file (the documented path).
const edited = JSON.parse(fs.readFileSync(file, 'utf-8'));
delete edited.disableWorkflows;
edited.env.CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS = '1';
fs.writeFileSync(file, JSON.stringify(edited, null, 2) + '\n');
initGroupFilesystem(ag, {}); // next spawn
const after = JSON.parse(fs.readFileSync(file, 'utf-8'));
expect(after.disableWorkflows).toBeUndefined();
expect(after.env.CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS).toBe('1');
});
});
-1
View File
@@ -11,7 +11,6 @@ const DEFAULT_SETTINGS_JSON =
JSON.stringify(
{
env: {
CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS: '1',
CLAUDE_CODE_ADDITIONAL_DIRECTORIES_CLAUDE_MD: '1',
CLAUDE_CODE_DISABLE_AUTO_MEMORY: '0',
},
+1
View File
@@ -18,6 +18,7 @@ export {
export {
ALLOW,
DENY,
GuardDenyError,
HOLD,
isUnguarded,
unguarded,
+13
View File
@@ -73,3 +73,16 @@ export const HOLD = (reason: string, approverUserId?: string): GuardDecision =>
reason,
approverUserId,
});
/**
* A guard deny travelling as an exception for flows whose entry point
* signals refusal by throwing (the a2a route). Catching it lets a caller
* distinguish "the guard refused, as designed" (report to the requester,
* log a warning) from a real runtime failure (crash path, stack trace).
*/
export class GuardDenyError extends Error {
constructor(reason: string) {
super(reason);
this.name = 'GuardDenyError';
}
}
+2 -2
View File
@@ -27,7 +27,7 @@ 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 { GuardDenyError, guard } from '../../guard/index.js';
import { log } from '../../log.js';
import { openInboundDb, resolveSession, sessionDir, writeSessionMessage } from '../../session-manager.js';
import type { PendingApproval, Session } from '../../types.js';
@@ -255,7 +255,7 @@ export async function routeAgentMessage(
});
if (decision.effect === 'deny') {
throw new Error(decision.reason);
throw new GuardDenyError(decision.reason);
}
// Gated edge: hold the message and return (not throw) so the delivery loop
+16 -11
View File
@@ -205,29 +205,33 @@ describe('agent message policies', () => {
expect(requestApproval).not.toHaveBeenCalled();
});
it('destination revoked between hold and approve → the approved replay is blocked', async () => {
it('destination revoked between hold and approve → refused cleanly, requester told, nothing delivered', 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/);
const notify = vi.fn();
// An expected policy refusal — resolves (no throw), so the response
// handler never records it as a handler crash.
await applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify, payload, approval });
expect(readInbound(B, SB.id)).toHaveLength(0);
expect(notify).toHaveBeenCalledWith(expect.stringMatching(/not delivered.*no destination for/));
});
it('mismatched grant (held for another target) refuses the replay', async () => {
it('mismatched grant (held for another target) refuses the replay cleanly', 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/);
const notify = vi.fn();
await applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify, payload, approval });
expect(readInbound(B, SB.id)).toHaveLength(0);
expect(notify).toHaveBeenCalledWith(expect.stringMatching(/not delivered.*invalid or mismatched grant/));
});
it('a grant only works while its row is live (executes once)', async () => {
@@ -237,10 +241,11 @@ describe('agent message policies', () => {
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/);
const notify = vi.fn();
await applyA2aMessageGate({ session: SA, userId: 'telegram:dana', notify, payload, approval });
expect(readInbound(B, SB.id)).toHaveLength(0);
expect(notify).toHaveBeenCalledWith(expect.stringMatching(/not delivered.*invalid or mismatched grant/));
});
// ── ghost-gate cleanup ──
+20 -8
View File
@@ -1,13 +1,10 @@
/** Approve handler for a held a2a message. (Reject is handled by the generic response-handler path.) */
import { GuardDenyError } from '../../guard/index.js';
import { log } from '../../log.js';
import type { ApprovalHandler } from '../approvals/index.js';
import { routeAgentMessage, type RoutableAgentMessage } from './agent-route.js';
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.');
@@ -24,10 +21,25 @@ export const applyA2aMessageGate: ApprovalHandler = async ({ session, payload, a
// One replay semantics: re-enter the guarded route carrying the approval
// row as the grant. The policy hold is satisfied, but the structural
// checks run 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 });
// checks run live — a deny here (destination revoked while the card was
// pending, dead or mismatched grant) is an EXPECTED policy outcome, not a
// crash: tell the requester, log a warning, and let anything else keep the
// response handler's failure path.
try {
await routeAgentMessage(msg, session, { grant: approval });
} catch (err) {
if (err instanceof GuardDenyError) {
log.warn('Approved a2a replay refused by the guard', {
from: session.agent_group_id,
to: platform_id,
msgId: msg.id,
reason: err.message,
});
notify(`Message approved, but not delivered — no longer authorized: ${err.message}`);
return;
}
throw err;
}
log.info('Held agent message delivered after approval', {
from: session.agent_group_id,
to: platform_id,
@@ -13,21 +13,12 @@ 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 { getDb } from '../../db/connection.js';
import { createMessagingGroup } from '../../db/messaging-groups.js';
import { createSession, createPendingApproval, getPendingApproval, getSession } from '../../db/sessions.js';
import { createSession, createPendingApproval } 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,
registerApprovalRequestedHandler,
registerApprovalResolvedHandler,
requestApproval,
type ApprovalRequestedEvent,
type ApprovalResolvedEvent,
} from './primitive.js';
import { registerApprovalHandler, registerApprovalResolvedHandler, type ApprovalResolvedEvent } from './primitive.js';
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
@@ -109,7 +100,7 @@ describe('approval-resolved callbacks', () => {
expect(events[0].outcome).toBe('reject');
expect(events[0].approval.approval_id).toBe('appr-reject-1');
expect(events[0].approval.action).toBe('test_reject_action');
expect(events[0].session?.id).toBe('sess-1');
expect(events[0].session.id).toBe('sess-1');
expect(events[0].userId).toBe('slack:admin-1');
});
@@ -159,43 +150,3 @@ 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();
});
});
-132
View File
@@ -1,132 +0,0 @@
/**
* mayResolve matrix the one click-authorization rule for every hold.
*
* These cases pin the folded rule to the behavior of the three pre-fold
* click-auth copies it replaces (approvals response handler, sender handler,
* channel handler), so unifying them changes no click decision:
* - 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)
*/
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 { upsertUser } from '../permissions/db/users.js';
import { grantRole } from '../permissions/db/user-roles.js';
import { approverRuleOf, mayResolve } from './approver-rule.js';
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, DELIVEREE)).toBe(true);
expect(mayResolve(e, OWNER)).toBe(false);
expect(mayResolve(e, GLOBAL_ADMIN)).toBe(false);
expect(mayResolve(e, SCOPED_ADMIN)).toBe(false);
expect(mayResolve(e, RANDO)).toBe(false);
expect(mayResolve(e, null)).toBe(false);
});
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, DELIVEREE)).toBe(true); // delivered-to shortcut
expect(mayResolve(e, SCOPED_ADMIN)).toBe(true);
expect(mayResolve(e, GLOBAL_ADMIN)).toBe(true);
expect(mayResolve(e, OWNER)).toBe(true);
expect(mayResolve(e, OTHER_ADMIN)).toBe(false); // admin of another group
expect(mayResolve(e, RANDO)).toBe(false);
expect(mayResolve(e, 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, SCOPED_ADMIN)).toBe(true);
expect(mayResolve(e, GLOBAL_ADMIN)).toBe(true);
expect(mayResolve(e, OWNER)).toBe(true);
expect(mayResolve(e, DELIVEREE)).toBe(false);
expect(mayResolve(e, 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, OWNER)).toBe(true);
expect(mayResolve(e, GLOBAL_ADMIN)).toBe(true);
expect(mayResolve(e, SCOPED_ADMIN)).toBe(false);
expect(mayResolve(e, 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, DELIVEREE)).toBe(true);
expect(mayResolve(e, SCOPED_ADMIN)).toBe(false);
});
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,
});
});
});
-59
View File
@@ -1,59 +0,0 @@
/**
* 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), 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`) / `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.
*
* `mayResolve` replaces the three divergent click-auth copies (approvals
* response handler, sender handler, channel handler) with one function.
*/
import type { ApproverRule, PendingApproval } from '../../types.js';
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
export type { ApproverRule } from '../../types.js';
/** May `clickerUserId` (namespaced `<channel>:<handle>`) resolve a hold with this approver rule? */
export function mayResolve(e: ApproverRule, clickerUserId: string | null): boolean {
if (!clickerUserId) return false;
if (e.kind === 'exclusive') {
return clickerUserId === e.approverUserId;
}
return (
(e.deliveredTo !== null && clickerUserId === e.deliveredTo) ||
(e.agentGroupId
? hasAdminPrivilege(clickerUserId, e.agentGroupId)
: isOwner(clickerUserId) || isGlobalAdmin(clickerUserId))
);
}
/** 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,
};
}
+14 -19
View File
@@ -25,31 +25,26 @@ 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 | null,
session: Session,
userId: string,
reason?: string,
): Promise<void> {
if (session) {
const text = reason
? `Your ${approval.action} request was rejected by admin: "${reason}"`
: `Your ${approval.action} request was rejected by admin.`;
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,
@@ -60,5 +55,5 @@ export async function finalizeReject(
deletePendingApproval(approval.approval_id);
await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
if (session) await wakeContainer(session);
await wakeContainer(session);
}
+3 -25
View File
@@ -16,21 +16,15 @@
*
* 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 { notifyApprovalRequested, notifyApprovalResolved, pickApprovalDelivery, pickApprover } from './primitive.js';
import { 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';
@@ -70,29 +64,20 @@ function shortApprovalId(): string {
return `oa-${Math.random().toString(36).slice(2, 10)}`;
}
/** 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 {
/** Called from the approvals response handler when a card button is clicked. */
export function resolveOneCLIApproval(approvalId: string, selectedOption: string): 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;
}
@@ -210,11 +195,6 @@ 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();
@@ -242,7 +222,6 @@ 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 });
}
@@ -272,7 +251,6 @@ 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: '' });
}
}
+24 -133
View File
@@ -23,12 +23,7 @@
*/
import { normalizeOptions, type RawOption } from '../../channels/ask-question.js';
import { getMessagingGroup } from '../../db/messaging-groups.js';
import {
createPendingApproval,
getPendingApproval,
getPendingApprovalByDedupKey,
getSession,
} from '../../db/sessions.js';
import { createPendingApproval, getSession } from '../../db/sessions.js';
import { getDeliveryAdapter } from '../../delivery.js';
import { wakeContainer } from '../../container-runner.js';
import { log } from '../../log.js';
@@ -62,8 +57,7 @@ const APPROVAL_OPTIONS: RawOption[] = [
// their `requestApproval()` calls.
export interface ApprovalHandlerContext {
/** Requesting agent's session. Null for sessionless holds (e.g. sender admission). */
session: Session | null;
session: Session;
payload: Record<string, unknown>;
/**
* The verified approval row the grant an approved continuation carries
@@ -73,7 +67,7 @@ export interface ApprovalHandlerContext {
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. No-op when sessionless. */
/** Send a system chat message to the requesting agent's session. */
notify: (text: string) => void;
}
@@ -100,20 +94,14 @@ 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
// 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'.
// isAuthorizedApprovalClick gate runs first), so callbacks never fire for
// unauthorized responses.
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;
/** 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). */
session: Session;
outcome: 'approve' | 'reject';
/** Namespaced user ID (`<channel>:<handle>`) of the resolving admin. Empty string if unknown. */
userId: string;
}
@@ -142,51 +130,6 @@ 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 ──
/**
@@ -263,14 +206,7 @@ export function notifyAgent(session: Session, text: string): void {
}
export interface RequestApprovalOptions {
/**
* 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;
session: Session;
agentName: string;
/** Free-form action identifier. Must match the key the consumer registered via registerApprovalHandler. */
action: string;
@@ -280,27 +216,8 @@ export interface RequestApprovalOptions {
title: string;
/** Card body shown to the admin. */
question: string;
/**
* Deliver the card to this specific user AND make the hold exclusively
* theirs to resolve (approver rule 'exclusive' an a2a policy's approver).
*/
/** Deliver the card to this specific user instead of all of the session group's admins. */
approverUserId?: string;
/** 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;
}
/**
@@ -310,63 +227,37 @@ 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, dedupKey } = opts;
const { session, action, payload, title, question, agentName, approverUserId } = opts;
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 approvers = approverUserId ? [approverUserId] : pickApprover(agentGroupId);
const approvers = approverUserId ? [approverUserId] : pickApprover(session.agent_group_id);
if (approvers.length === 0) {
fail('no owner or admin configured to approve.');
notifyAgent(session, `${action} failed: no owner or admin configured to approve.`);
return;
}
const originChannelType =
opts.originChannelType ??
(session?.messaging_group_id ? (getMessagingGroup(session.messaging_group_id)?.channel_type ?? '') : '');
const originChannelType = session.messaging_group_id
? (getMessagingGroup(session.messaging_group_id)?.channel_type ?? '')
: '';
const target = await pickApprovalDelivery(approvers, originChannelType);
if (!target) {
fail('no DM channel found for any eligible approver.');
notifyAgent(session, `${action} failed: no DM channel found for any eligible approver.`);
return;
}
const approvalId = `appr-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
const cardOptions = opts.options ?? APPROVAL_OPTIONS;
const inserted = createPendingApproval({
const normalizedOptions = normalizeOptions(APPROVAL_OPTIONS);
createPendingApproval({
approval_id: approvalId,
session_id: session?.id ?? null,
session_id: session.id,
request_id: approvalId,
action,
payload: JSON.stringify(payload),
created_at: new Date().toISOString(),
agent_group_id: agentGroupId,
title,
options_json: JSON.stringify(normalizeOptions(cardOptions)),
approver_user_id: approverUserId ?? (opts.recordDeliveredApprover ? target.userId : null),
approver_rule: approverUserId ? 'exclusive' : 'admins-of-scope',
dedup_key: dedupKey ?? null,
options_json: JSON.stringify(normalizedOptions),
approver_user_id: approverUserId ?? null,
});
if (!inserted) {
// A concurrent request with the same dedup key won the partial UNIQUE —
// this insert was dropped, so don't announce or deliver a second card.
log.debug('Approval request already in flight — dropping duplicate', { action, dedupKey });
return;
}
const created = getPendingApproval(approvalId);
if (created) {
await notifyApprovalRequested({ approval: created, session: session ?? null, deliveredTo: target.userId });
}
const adapter = getDeliveryAdapter();
if (adapter) {
@@ -381,12 +272,12 @@ export async function requestApproval(opts: RequestApprovalOptions): Promise<voi
questionId: approvalId,
title,
question,
options: cardOptions,
options: APPROVAL_OPTIONS,
}),
);
} catch (err) {
log.error('Failed to deliver approval card', { action, approvalId, err });
fail(`could not deliver approval request to ${target.userId}.`);
notifyAgent(session, `${action} failed: could not deliver approval request to ${target.userId}.`);
return;
}
}
+44 -32
View File
@@ -12,9 +12,6 @@
* 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-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.
*/
@@ -23,8 +20,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, Session } from '../../types.js';
import { approverRuleOf, mayResolve } from './approver-rule.js';
import type { PendingApproval } from '../../types.js';
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
import { finalizeReject } from './finalize.js';
import { ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
import { getApprovalHandler, notifyApprovalResolved, REJECT_WITH_REASON_VALUE } from './primitive.js';
@@ -34,8 +31,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
const approval = getPendingApproval(payload.questionId);
if (!approval) return false;
const clickerId = namespacedUserId(payload);
if (!mayResolve(approverRuleOf(approval), clickerId)) {
if (!isAuthorizedApprovalClick(approval, payload)) {
log.warn('Ignoring unauthorized approval response', {
approvalId: approval.approval_id,
action: approval.action,
@@ -46,7 +42,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
}
if (approval.action === ONECLI_ACTION) {
if (resolveOneCLIApproval(payload.questionId, payload.value, clickerId ?? '')) {
if (resolveOneCLIApproval(payload.questionId, payload.value)) {
return true;
}
// Row exists but the in-memory resolver is gone (timer fired or the process
@@ -55,7 +51,7 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
return true;
}
await handleRegisteredApproval(approval, payload.value, clickerId ?? '');
await handleRegisteredApproval(approval, payload.value, namespacedUserId(payload) ?? '');
return true;
}
@@ -64,11 +60,12 @@ async function handleRegisteredApproval(
selectedOption: string,
userId: string,
): Promise<void> {
// 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) {
if (!approval.session_id) {
deletePendingApproval(approval.approval_id);
return;
}
const session = getSession(approval.session_id);
if (!session) {
deletePendingApproval(approval.approval_id);
return;
}
@@ -76,10 +73,8 @@ 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) {
if (session) await armReasonCapture(approval, session, userId);
else await finalizeReject(approval, null, userId);
await armReasonCapture(approval, session, userId);
return;
}
@@ -90,19 +85,17 @@ async function handleRegisteredApproval(
}
// Approved — dispatch to the module that registered for this action.
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 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 handler = getApprovalHandler(approval.action);
if (!handler) {
@@ -113,7 +106,7 @@ async function handleRegisteredApproval(
notify(`Your ${approval.action} was approved, but no handler is installed to apply it.`);
deletePendingApproval(approval.approval_id);
await notifyApprovalResolved({ approval, session, outcome: 'approve', userId });
if (session) await wakeContainer(session);
await wakeContainer(session);
return;
}
@@ -130,10 +123,29 @@ async function handleRegisteredApproval(
deletePendingApproval(approval.approval_id);
await notifyApprovalResolved({ approval, session, outcome: 'approve', userId });
if (session) await wakeContainer(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);
}
+338 -20
View File
@@ -19,9 +19,23 @@ import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
import { initTestDb, closeDb, runMigrations } from '../../db/index.js';
import { createAgentGroup } from '../../db/agent-groups.js';
import { createMessagingGroup, getMessagingGroupByPlatform } from '../../db/messaging-groups.js';
import { registerChannelAdapter } from '../../channels/channel-registry.js';
import type { ChannelDefaults } from '../../channels/adapter.js';
import { upsertUser } from './db/users.js';
import { grantRole } from './db/user-roles.js';
// Registration-tier declaration for the fixture channel — a threaded platform
// whose declared defaults match the historical card-flow behavior
// (mention-sticky groups, pattern '.' DMs). Without it, the no-live-adapter
// fallback resolves threads=false and coerces sticky → mention.
// Registry maps are module-global; keep channel names unique per test file.
const telegramDefaults: ChannelDefaults = {
dm: { engageMode: 'pattern', engagePattern: '.', threads: true, unknownSenderPolicy: 'request_approval' },
group: { engageMode: 'mention-sticky', threads: true, unknownSenderPolicy: 'request_approval' },
mentions: 'platform',
};
registerChannelAdapter('telegram', { factory: () => null, defaults: telegramDefaults });
// Mock container runner — prevent actual docker spawn.
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
@@ -54,7 +68,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';
@@ -113,13 +131,14 @@ function groupMention(platformId: string, text = '@bot hello') {
return {
channelType: 'telegram',
platformId,
threadId: 'thread-1', // non-null → is_group=true per channel-approval default-picker logic
threadId: 'thread-1',
message: {
id: `msg-${Math.random().toString(36).slice(2, 8)}`,
kind: 'chat' as const,
content: JSON.stringify({ senderId: 'caller', senderName: 'Caller', text }),
timestamp: now(),
isMention: true,
isGroup: true, // group context comes from the adapter flag, never threadId
},
};
}
@@ -153,6 +172,8 @@ describe('unknown-channel registration flow', () => {
expect(kind).toBe('chat-sdk');
const payload = JSON.parse(content as string);
expect(payload.type).toBe('ask_question');
// Card tells the approver the resolved engage rule.
expect(payload.question).toContain('will respond to @-mentions in this group');
// Single-agent card offers a direct "Connect to <name>" button.
const connectOption = payload.options.find((o: { value: string }) => o.value.startsWith('connect:'));
expect(connectOption).toBeDefined();
@@ -171,28 +192,13 @@ describe('unknown-channel registration flow', () => {
await new Promise((r) => setTimeout(r, 10));
expect(deliverMock).toHaveBeenCalledTimes(1);
const payload = JSON.parse(deliverMock.mock.calls[0][4] as string) as { question: string };
expect(payload.question).toContain('will respond to all messages');
const { getDb } = await import('../../db/connection.js');
const count = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_channel_approvals').get() as { c: number }).c;
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'));
@@ -245,7 +251,7 @@ describe('unknown-channel registration flow', () => {
agent_group_id: string;
};
expect(mga).toBeDefined();
expect(mga.engage_mode).toBe('mention-sticky'); // group (threadId != null)
expect(mga.engage_mode).toBe('mention-sticky'); // declared group default (threads:true keeps sticky)
expect(mga.engage_pattern).toBeNull();
expect(mga.sender_scope).toBe('known');
expect(mga.ignored_message_policy).toBe('accumulate');
@@ -296,6 +302,135 @@ describe('unknown-channel registration flow', () => {
expect(mga.engage_pattern).toBe('.');
});
// WhatsApp-like platform: groups exist but thread ids don't (threadId is
// always null), and the adapter is undeclared (stale skill-installed copy)
// so resolution goes through the behavior-faithful fallback. This is the
// one deliberate behavior change of the defaults work: the old
// `threadId !== null` heuristic misread these groups as DMs and wired
// pattern '.'.
function waGroupMention(platformId: string) {
return {
channelType: 'wamock',
platformId,
threadId: null,
message: {
id: `msg-${Math.random().toString(36).slice(2, 8)}`,
kind: 'chat' as const,
content: JSON.stringify({ senderId: 'caller', senderName: 'Caller', text: '@bot hi' }),
timestamp: now(),
isMention: true,
isGroup: true,
},
};
}
async function approvePending(agentGroupId = 'ag-1') {
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
const pending = getDb().prepare('SELECT messaging_group_id FROM pending_channel_approvals').get() as {
messaging_group_id: string;
};
expect(pending).toBeDefined();
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending.messaging_group_id,
value: `connect:${agentGroupId}`,
userId: 'owner',
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
});
if (claimed) break;
}
return pending.messaging_group_id;
}
it('non-threaded group (isGroup flag, null threadId) wires the GROUP default, sticky coerced to mention', async () => {
const { routeInbound } = await import('../../router.js');
await routeInbound(waGroupMention('wa-group-1'));
await new Promise((r) => setTimeout(r, 10));
const mgId = await approvePending();
const { getDb } = await import('../../db/connection.js');
const mga = getDb()
.prepare('SELECT engage_mode, engage_pattern FROM messaging_group_agents WHERE messaging_group_id = ?')
.get(mgId) as { engage_mode: string; engage_pattern: string | null };
// Faithful fallback group default is mention-sticky, but with no live
// adapter threads resolve false → coerced to plain mention. NOT the old
// pattern '.' DM misclassification.
expect(mga.engage_mode).toBe('mention');
expect(mga.engage_pattern).toBeNull();
});
it('DM on an undeclared channel stays pattern "." through the faithful fallback', async () => {
const { routeInbound } = await import('../../router.js');
await routeInbound({
...waGroupMention('wa-dm-1'),
message: { ...waGroupMention('wa-dm-1').message, isGroup: false },
});
await new Promise((r) => setTimeout(r, 10));
const mgId = await approvePending();
const { getDb } = await import('../../db/connection.js');
const mga = getDb()
.prepare('SELECT engage_mode, engage_pattern FROM messaging_group_agents WHERE messaging_group_id = ?')
.get(mgId) as { engage_mode: string; engage_pattern: string | null };
expect(mga.engage_mode).toBe('pattern');
expect(mga.engage_pattern).toBe('.');
});
it('connect-existing and new-agent approve paths produce identical wirings', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
// Path 1: connect to existing agent.
await routeInbound(groupMention('chat-path-connect'));
await new Promise((r) => setTimeout(r, 10));
const mgIdConnect = await approvePending();
// Path 2: new agent via free-text name reply.
await routeInbound(groupMention('chat-path-newagent'));
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;
}
// Owner replies with the agent name in their DM — interceptor wires.
await routeInbound({
channelType: 'telegram',
platformId: 'dm-owner',
threadId: null,
message: {
id: `msg-${Math.random().toString(36).slice(2, 8)}`,
kind: 'chat' as const,
content: JSON.stringify({ senderId: 'owner', text: 'Bravo' }),
timestamp: now(),
},
});
await new Promise((r) => setTimeout(r, 10));
const select =
'SELECT engage_mode, engage_pattern, sender_scope, ignored_message_policy, session_mode, priority ' +
'FROM messaging_group_agents WHERE messaging_group_id = ?';
const viaConnect = getDb().prepare(select).get(mgIdConnect);
const viaNewAgent = getDb().prepare(select).get(pending.messaging_group_id);
expect(viaNewAgent).toBeDefined();
expect(viaNewAgent).toEqual(viaConnect);
});
it('deny → sets denied_at; future mentions drop silently without a second card', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
@@ -374,6 +509,189 @@ describe('unknown-channel registration flow', () => {
.c;
expect(stillPending).toBe(1);
});
it('does not let a scoped admin connect an unknown channel to another agent group', async () => {
const { routeInbound } = await import('../../router.js');
const { getResponseHandlers } = await import('../../response-registry.js');
const { getDb } = await import('../../db/connection.js');
createAgentGroup({ id: 'ag-2', name: 'Betty', folder: 'betty', agent_provider: null, created_at: now() });
upsertUser({ id: 'telegram:scoped-admin', kind: 'telegram', display_name: 'Scoped Admin', created_at: now() });
grantRole({
user_id: 'telegram:scoped-admin',
role: 'admin',
agent_group_id: 'ag-1',
granted_by: 'telegram:owner',
granted_at: now(),
});
createMessagingGroup({
id: 'mg-dm-scoped-admin',
channel_type: 'telegram',
platform_id: 'dm-scoped-admin',
name: 'Scoped 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('telegram:scoped-admin', 'telegram', 'mg-dm-scoped-admin', now());
await routeInbound(groupMention('chat-scoped-cross-group'));
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;
};
expect(pending).toBeDefined();
expect(deliverMock).toHaveBeenCalledTimes(1);
expect(deliverMock.mock.calls[0][1]).toBe('dm-scoped-admin');
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;
}
const mgaCount = (
getDb()
.prepare('SELECT COUNT(*) AS c FROM messaging_group_agents WHERE messaging_group_id = ?')
.get(pending.messaging_group_id) as { c: number }
).c;
expect(mgaCount).toBe(0);
const stillPending = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_channel_approvals').get() as { c: number })
.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 interceptor
// captures it 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('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 interceptor 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', () => {
+5 -43
View File
@@ -53,13 +53,9 @@ 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, PendingApproval } from '../../types.js';
import { notifyApprovalRequested, pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
import {
createPendingChannelApproval,
hasInFlightChannelApproval,
type PendingChannelApproval,
} from './db/pending-channel-approvals.js';
import type { AgentGroup } from '../../types.js';
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
import { createPendingChannelApproval, hasInFlightChannelApproval } from './db/pending-channel-approvals.js';
import { hasAdminPrivilege } from './db/user-roles.js';
// ── Value constants (response handler in index.ts parses these) ──
@@ -183,7 +179,6 @@ 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];
@@ -250,7 +245,7 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
const question = buildQuestionText(isGroup, senderName, channelName, originChannelType, ruleNote);
const options = normalizeOptions(buildApprovalOptions(agentGroups, delivery.userId));
const row: PendingChannelApproval = {
createPendingChannelApproval({
messaging_group_id: messagingGroupId,
agent_group_id: referenceGroup.id,
original_message: JSON.stringify(event),
@@ -258,9 +253,7 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput)
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) {
@@ -315,37 +308,6 @@ 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',
dedup_key: null,
};
}
/**
* Create a new agent group and initialize its filesystem. Handles
* folder-name collisions with numeric suffixes.
@@ -0,0 +1,60 @@
/**
* 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);
}
+227 -221
View File
@@ -17,8 +17,8 @@
*/
import { recordDroppedMessage } from '../../db/dropped-messages.js';
import { getAgentGroup, getAllAgentGroups } from '../../db/agent-groups.js';
import { createMessagingGroupAgent, setMessagingGroupDeniedAt } from '../../db/messaging-groups.js';
import { deletePendingApproval, getPendingApprovalByDedupKey } from '../../db/sessions.js';
import { createMessagingGroupAgent, getMessagingGroup, setMessagingGroupDeniedAt } from '../../db/messaging-groups.js';
import { resolveWiringDefaults } from '../../channels/channel-defaults.js';
import {
routeInbound,
setAccessGate,
@@ -33,18 +33,11 @@ import { registerResponseHandler, type ResponsePayload } from '../../response-re
import { getDeliveryAdapter } from '../../delivery.js';
import { log } from '../../log.js';
import type { MessagingGroup, MessagingGroupAgent } from '../../types.js';
import { mayResolve } from '../approvals/approver-rule.js';
import { notifyApprovalResolved, registerApprovalHandler } from '../approvals/primitive.js';
import { guard } from '../../guard/index.js';
import { channelsRegister, sendersAdmit } from './guard.js';
import { canAccessAgentGroup } from './access.js';
// Registers the permissions guard catalog entries (senders.admit, channels.register)
// via defineGuardedAction at module load — keeps the guard catalog complete. Their
// decision bodies stand as the policy-as-data seam; unknown-sender admission and
// channel-registration click-auth are currently enforced inline below (unknown_sender_policy
// switch + mayResolve), which also carries the approval-lifecycle observer wiring.
import './guard.js';
import {
buildAgentSelectionOptions,
channelHoldView,
CHOOSE_EXISTING_VALUE,
CONNECT_PREFIX,
createNewAgentGroup,
@@ -57,10 +50,12 @@ import {
deletePendingChannelApproval,
getPendingChannelApproval,
updatePendingChannelApprovalCard,
type PendingChannelApproval,
} 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, senderAdmitDedupKey, SENDER_ADMIT_ACTION } from './sender-approval.js';
import { requestSenderApproval } from './sender-approval.js';
import { ensureUserDm } from './user-dm.js';
// ── Free-text name input state ──
@@ -138,43 +133,49 @@ 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 decision (./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(sendersAdmit, {
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);
@@ -218,47 +219,83 @@ setSenderScopeGate(
);
/**
* 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.
* Response handler for the unknown-sender approval card.
*
* 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.
* 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").
*/
registerApprovalHandler(SENDER_ADMIT_ACTION, async ({ payload, userId }) => {
const senderIdentity = typeof payload.senderIdentity === 'string' ? payload.senderIdentity : '';
const agentGroupId = typeof payload.agentGroupId === 'string' ? payload.agentGroupId : '';
const messagingGroupId = typeof payload.messagingGroupId === 'string' ? payload.messagingGroupId : '';
if (!senderIdentity || !agentGroupId) {
log.warn('sender_admit approved but the hold payload was malformed', { senderIdentity, agentGroupId });
return;
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;
}
addMember({
user_id: senderIdentity,
agent_group_id: agentGroupId,
added_by: userId,
added_at: new Date().toISOString(),
log.info('Unknown sender denied', {
approvalId: row.id,
senderIdentity: row.sender_identity,
agentGroupId: row.agent_group_id,
approverId,
});
log.info('Unknown sender approved — member added', { senderIdentity, agentGroupId, approverId: userId });
deletePendingSenderApproval(row.id);
return true;
}
// Delete the hold BEFORE re-routing (the pre-fold table did the same). The
// replay re-fans-out the original event, and any OTHER agent group on this
// messaging group must be free to mint its own sender card — the shared
// dedup key would suppress it while this now-resolved row still exists.
if (messagingGroupId) {
const hold = getPendingApprovalByDedupKey(senderAdmitDedupKey(messagingGroupId, senderIdentity));
if (hold) deletePendingApproval(hold.approval_id);
}
try {
await routeInbound(payload.event as InboundEvent);
} catch (err) {
log.error('Failed to replay message after sender approval', { senderIdentity, agentGroupId, err });
}
});
registerResponseHandler(handleSenderApprovalResponse);
// ── Unknown-channel registration flow ──
@@ -266,6 +303,104 @@ setChannelRequestGate(async (mg, event) => {
await requestChannelApproval({ messagingGroupId: mg.id, event });
});
/**
* Wire an approved channel to an agent group and replay the stored event.
* Shared by both approve paths (connect-existing button, free-text new-agent
* name reply) so they produce identical wirings. Returns true when the wiring
* was created callers must not confirm success to the approver otherwise.
*
* Engage defaults come from the channel's declared defaults (DM vs group
* context). isGroup uses the adapter's own flag with the persisted row as
* fallback never `threadId !== null` (DM sub-threads exist on Slack/Discord;
* non-threaded group platforms like WhatsApp have null threadIds in groups).
*/
async function wireApprovedChannel(
row: PendingChannelApproval,
agentGroupId: string,
approverId: string,
): Promise<boolean> {
let event: InboundEvent;
try {
event = JSON.parse(row.original_message) as InboundEvent;
} catch (err) {
log.error('Channel registration: failed to parse stored event', {
messagingGroupId: row.messaging_group_id,
err,
});
deletePendingChannelApproval(row.messaging_group_id);
return false;
}
const mg = getMessagingGroup(row.messaging_group_id);
const isGroup = event.message.isGroup ?? mg?.is_group === 1;
const agentGroupName = getAgentGroup(agentGroupId)?.name ?? '';
let engage: { engage_mode: MessagingGroupAgent['engage_mode']; engage_pattern: string | null };
try {
engage = resolveWiringDefaults(
mg?.instance ?? mg?.channel_type ?? event.channelType,
isGroup,
agentGroupName,
mg?.channel_type ?? event.channelType,
);
} catch (err) {
// Mis-declared adapter (pattern mode without a pattern). Drop the pending
// row so a future mention can retry once the declaration is fixed.
log.error('Channel registration: channel defaults unresolvable', {
messagingGroupId: row.messaging_group_id,
err,
});
deletePendingChannelApproval(row.messaging_group_id);
return false;
}
const mgaId = `mga-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
createMessagingGroupAgent({
id: mgaId,
messaging_group_id: row.messaging_group_id,
agent_group_id: agentGroupId,
engage_mode: engage.engage_mode,
engage_pattern: engage.engage_pattern,
// Deliberate card-flow choices, not channel defaults: the triggering
// sender is auto-admitted below, so 'known' keeps other strangers gated;
// 'accumulate' / 'shared' / priority 0 are the flow's fixed semantics.
sender_scope: 'known',
ignored_message_policy: 'accumulate',
session_mode: 'shared',
priority: 0,
created_at: new Date().toISOString(),
});
log.info('Channel registration approved — wiring created', {
messagingGroupId: row.messaging_group_id,
agentGroupId,
mgaId,
engageMode: engage.engage_mode,
approverId,
});
const senderUserId = extractAndUpsertUser(event);
if (senderUserId) {
addMember({
user_id: senderUserId,
agent_group_id: agentGroupId,
added_by: approverId,
added_at: new Date().toISOString(),
});
}
deletePendingChannelApproval(row.messaging_group_id);
try {
await routeInbound(event);
} catch (err) {
log.error('Failed to replay message after channel approval', {
messagingGroupId: row.messaging_group_id,
err,
});
}
return true;
}
/**
* Response handler for the unknown-channel registration card.
*
@@ -284,41 +419,32 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
const row = getPendingChannelApproval(payload.questionId);
if (!row) return false;
// payload.userId is the raw platform userId (e.g. "6037840640"); namespace it
// with the channel type so it matches users(id) format. Some platforms
// (e.g. Teams "29:xxx") already include a colon — mirror resolveOrCreateUser
// logic and only prefix when the raw id has no colon.
// Click authorization is the guard's channels.register decision (./guard.ts):
// the delivered approver, or an admin of the pending row's anchor agent group.
const clickerId = payload.userId
? payload.userId.includes(':')
? payload.userId
: `${payload.channelType}:${payload.userId}`
: null;
// The delivered-to approver, or an admin of the hold's anchoring agent group.
if (
!mayResolve(
{ kind: 'admins-of-scope', agentGroupId: row.agent_group_id, deliveredTo: row.approver_user_id },
clickerId,
)
) {
const decision = guard(channelsRegister, {
actor: { kind: 'human', userId: clickerId ?? '' },
payload: { questionId: payload.questionId },
});
if (!clickerId || decision.effect !== 'allow') {
log.warn('Channel registration click rejected — unauthorized clicker', {
messagingGroupId: row.messaging_group_id,
clickerId,
expectedApprover: row.approver_user_id,
reason: decision.reason,
});
return true;
}
const approverId = clickerId as string;
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,
@@ -442,69 +568,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise<
}
// ── Wire + replay (shared path for connect and create) ──
let event: InboundEvent;
try {
event = JSON.parse(row.original_message) as InboundEvent;
} catch (err) {
log.error('Channel registration: failed to parse stored event', {
messagingGroupId: row.messaging_group_id,
err,
});
deletePendingChannelApproval(row.messaging_group_id);
return true;
}
const isGroup = event.threadId !== null;
const engageMode: MessagingGroupAgent['engage_mode'] = isGroup ? 'mention-sticky' : 'pattern';
const engagePattern = isGroup ? null : '.';
const mgaId = `mga-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
createMessagingGroupAgent({
id: mgaId,
messaging_group_id: row.messaging_group_id,
agent_group_id: targetAgentGroupId,
engage_mode: engageMode,
engage_pattern: engagePattern,
sender_scope: 'known',
ignored_message_policy: 'accumulate',
session_mode: 'shared',
priority: 0,
created_at: new Date().toISOString(),
});
log.info('Channel registration approved — wiring created', {
messagingGroupId: row.messaging_group_id,
agentGroupId: targetAgentGroupId,
mgaId,
engageMode,
approverId,
});
const senderUserId = extractAndUpsertUser(event);
if (senderUserId) {
addMember({
user_id: senderUserId,
agent_group_id: targetAgentGroupId,
added_by: approverId,
added_at: new Date().toISOString(),
});
}
deletePendingChannelApproval(row.messaging_group_id);
await notifyApprovalResolved({
approval: channelHoldView(row, { targetAgentGroupId }),
session: null,
outcome: 'approve',
userId: approverId,
});
try {
await routeInbound(event);
} catch (err) {
log.error('Failed to replay message after channel approval', {
messagingGroupId: row.messaging_group_id,
err,
});
}
await wireApprovedChannel(row, targetAgentGroupId, approverId);
return true;
}
@@ -548,69 +612,7 @@ registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
folder: ag.folder,
});
let originalEvent: InboundEvent;
try {
originalEvent = JSON.parse(row.original_message) as InboundEvent;
} catch (err) {
log.error('Channel registration: failed to parse stored event', {
messagingGroupId: row.messaging_group_id,
err,
});
deletePendingChannelApproval(row.messaging_group_id);
return true;
}
const isGroup = originalEvent.threadId !== null;
const engageMode: MessagingGroupAgent['engage_mode'] = isGroup ? 'mention-sticky' : 'pattern';
const engagePattern = isGroup ? null : '.';
const mgaId = `mga-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
createMessagingGroupAgent({
id: mgaId,
messaging_group_id: row.messaging_group_id,
agent_group_id: ag.id,
engage_mode: engageMode,
engage_pattern: engagePattern,
sender_scope: 'known',
ignored_message_policy: 'accumulate',
session_mode: 'shared',
priority: 0,
created_at: new Date().toISOString(),
});
log.info('Channel registration approved — wiring created', {
messagingGroupId: row.messaging_group_id,
agentGroupId: ag.id,
mgaId,
engageMode,
approverId: userId,
});
const senderUserId = extractAndUpsertUser(originalEvent);
if (senderUserId) {
addMember({
user_id: senderUserId,
agent_group_id: ag.id,
added_by: userId,
added_at: new Date().toISOString(),
});
}
deletePendingChannelApproval(row.messaging_group_id);
await notifyApprovalResolved({
approval: channelHoldView(row, { targetAgentGroupId: ag.id, createdAgentGroup: true }),
session: null,
outcome: 'approve',
userId,
});
try {
await routeInbound(originalEvent);
} catch (err) {
log.error('Failed to replay message after channel approval', {
messagingGroupId: row.messaging_group_id,
err,
});
}
const wired = await wireApprovedChannel(row, ag.id, userId);
const adapter = getDeliveryAdapter();
if (adapter) {
@@ -622,7 +624,11 @@ registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
dm.platform_id,
null,
'chat-sdk',
JSON.stringify({ text: `✅ Agent "${ag.name}" created and connected.` }),
JSON.stringify({
text: wired
? `✅ Agent "${ag.name}" created and connected.`
: `⚠️ Agent "${ag.name}" was created but the channel couldn't be connected — check the host logs.`,
}),
)
.catch(() => {});
}
+36 -67
View File
@@ -1,8 +1,6 @@
/**
* Integration tests for the unknown-sender request_approval flow
* (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.
* (ACTION-ITEMS item 5).
*
* Covers:
* - request_approval policy fires `requestSenderApproval` on first unknown
@@ -11,9 +9,7 @@
* silently dropped (no second card, no second row)
* - Approve path: member added, original message replayed via routeInbound,
* container woken
* - 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
* - Deny path: pending row deleted, no member added
*/
import fs from 'fs';
import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
@@ -32,14 +28,12 @@ vi.mock('../../container-runner.js', () => ({
killContainer: vi.fn(),
}));
// Mock delivery adapter — record card deliveries for assertions. The approvals
// barrel also pulls onDeliveryAdapterReady from this module at import time.
// Mock delivery adapter — record card deliveries for assertions.
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
@@ -75,12 +69,10 @@ beforeEach(async () => {
const db = initTestDb();
runMigrations(db);
// 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.
// 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.
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.
@@ -160,20 +152,6 @@ 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');
@@ -190,26 +168,11 @@ 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(/^appr-/);
expect(payload.title).toBe('👤 New sender');
expect(payload.options.map((o: { value: string }) => o.value)).toEqual(['approve', 'reject']);
expect(payload.questionId).toMatch(/^nsa-/);
const { getDb } = await import('../../db/connection.js');
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;
}>;
const rows = getDb().prepare('SELECT * FROM pending_sender_approvals').all();
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 () => {
@@ -220,7 +183,9 @@ describe('unknown-sender request_approval flow', () => {
await new Promise((r) => setTimeout(r, 10));
expect(deliverMock).toHaveBeenCalledTimes(1);
expect(await senderHoldCount()).toBe(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);
});
it('approve → adds member and replays the original message', async () => {
@@ -232,16 +197,17 @@ describe('unknown-sender request_approval flow', () => {
await routeInbound(stranger('please let me in'));
await new Promise((r) => setTimeout(r, 10));
const pending = await pendingSenderHold();
const { getDb } = await import('../../db/connection.js');
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
expect(pending).toBeDefined();
// Fire the approve click through the response-handler chain.
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending!.approval_id,
questionId: pending.id,
value: 'approve',
// Chat SDK's onAction surfaces the raw platform userId (e.g. Telegram
// chat id). The response handler namespaces it with channelType to
// chat id). The permissions handler namespaces it with channelType to
// match users(id).
userId: 'owner',
channelType: 'telegram',
@@ -252,32 +218,33 @@ 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 hold cleared.
expect(await senderHoldCount()).toBe(0);
// Pending row cleared.
const stillPending = getDb().prepare('SELECT COUNT(*) AS c FROM pending_sender_approvals').get() as { c: number };
expect(stillPending.c).toBe(0);
// Message replayed + container woken.
expect(wakeContainer).toHaveBeenCalled();
});
it('deny → deletes the pending hold without adding a member', async () => {
it('deny → deletes the pending row 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 pending = await pendingSenderHold();
const { getDb } = await import('../../db/connection.js');
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
expect(pending).toBeDefined();
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending!.approval_id,
questionId: pending.id,
value: 'reject',
userId: 'owner', // raw platform id — handler namespaces with channelType
channelType: 'telegram',
@@ -287,8 +254,8 @@ describe('unknown-sender request_approval flow', () => {
if (claimed) break;
}
expect(await senderHoldCount()).toBe(0);
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(0);
const member = getDb()
.prepare('SELECT 1 AS x FROM agent_group_members WHERE user_id = ? AND agent_group_id = ?')
.get('tg:stranger', 'ag-1');
@@ -303,7 +270,8 @@ describe('unknown-sender request_approval flow', () => {
await routeInbound(stranger('can I play'));
await new Promise((r) => setTimeout(r, 10));
const pending = await pendingSenderHold();
const { getDb } = await import('../../db/connection.js');
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
expect(pending).toBeDefined();
// A random user (not the stranger, not the owner, not an admin) tries to
@@ -311,7 +279,7 @@ describe('unknown-sender request_approval flow', () => {
// rejected without admitting them.
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending!.approval_id,
questionId: pending.id,
value: 'approve',
userId: 'random-bystander', // not owner, not admin
channelType: 'telegram',
@@ -322,17 +290,18 @@ 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 hold is still there — a legitimate approver can still act on it.
expect(await senderHoldCount()).toBe(1);
// 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);
});
it('accepts a click from a global admin even if they are not the delivered approver', async () => {
it('accepts a click from a global admin even if they are not the designated 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({
@@ -349,13 +318,14 @@ describe('unknown-sender request_approval flow', () => {
await routeInbound(stranger('knock knock'));
await new Promise((r) => setTimeout(r, 10));
const pending = await pendingSenderHold();
const { getDb } = await import('../../db/connection.js');
const pending = getDb().prepare('SELECT id FROM pending_sender_approvals').get() as { id: string };
expect(pending).toBeDefined();
// Admin clicks approve (not the delivered approver, which was the owner).
// Admin clicks approve (not the designated approver, which was owner).
for (const handler of getResponseHandlers()) {
const claimed = await handler({
questionId: pending!.approval_id,
questionId: pending.id,
value: 'approve',
userId: 'admin-bob',
channelType: 'telegram',
@@ -366,7 +336,6 @@ 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');
+123 -35
View File
@@ -3,38 +3,44 @@
*
* 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`, which holds through the
* approvals primitive (action 'sender_admit'):
* attempt and calls `requestSenderApproval` to:
*
* - 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.
* 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.
*
* 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).
* 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).
*/
import type { RawOption } from '../../channels/ask-question.js';
import { normalizeOptions, 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 { requestApproval } from '../approvals/primitive.js';
import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js';
import { createPendingSenderApproval, hasInFlightSenderApproval } from './db/pending-sender-approvals.js';
const APPROVAL_OPTIONS: RawOption[] = [
{ label: 'Allow', selectedLabel: '✅ Allowed', value: 'approve', style: 'primary' },
{ label: 'Deny', selectedLabel: '❌ Denied', value: 'reject', style: 'danger' },
];
export const SENDER_ADMIT_ACTION = 'sender_admit';
export function senderAdmitDedupKey(messagingGroupId: string, senderIdentity: string): string {
return `${SENDER_ADMIT_ACTION}:${messagingGroupId}:${senderIdentity}`;
function generateId(): string {
return `nsa-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
}
export interface RequestSenderApprovalInput {
@@ -48,20 +54,102 @@ export interface RequestSenderApprovalInput {
export async function requestSenderApproval(input: RequestSenderApprovalInput): Promise<void> {
const { messagingGroupId, agentGroupId, senderIdentity, senderName, event } = input;
const originMg = getMessagingGroup(messagingGroupId);
const senderDisplay = senderName && senderName.length > 0 ? senderName : senderIdentity;
const originName = originMg?.name ?? `a ${originMg?.channel_type ?? ''} channel`;
// 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;
}
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 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 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),
});
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';
+1 -20
View File
@@ -198,17 +198,6 @@ 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 };
export interface PendingApproval {
approval_id: string;
session_id: string | null;
@@ -229,16 +218,8 @@ export interface PendingApproval {
status: 'pending' | 'approved' | 'rejected' | 'expired' | 'awaiting_reason';
title: string;
options_json: string;
/**
* 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.
*/
/** When set, only this exact user may resolve the approval. */
approver_user_id: string | null;
/** Who may resolve this hold — see modules/approvals/approver-rule.ts. */
approver_rule: 'exclusive' | 'admins-of-scope';
/** 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) ──