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>
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>
Translate the per-root `readOnly` key (and tolerate the top-level
`nonMainReadOnly` key) that /manage-mounts and setup actually write, so
read-write grants are no longer silently forced read-only. Read+validate
the allowlist per call (mtime-keyed cache) instead of caching it — and its
parse errors — for the whole process lifetime. Add tests and fix the
skill docs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Approval cards render every button with the same neutral style, so Approve
and Reject read identically at a glance. The chat SDK's Button already
accepts style ('primary' | 'danger' | 'default') and @chat-adapter/slack
maps primary→green and danger→red Block Kit styles (Telegram ignores it),
but the ask_question pipeline dropped the field.
- ask-question.ts: add OptionStyle and an optional style field to
OptionInput/NormalizedOption; normalizeOption whitelists the value.
Bare-string options stay unstyled.
- chat-sdk-bridge.ts: forward opt.style into Button() on ask_question
cards. Persisted options_json tolerates the extra key
(getAskQuestionRender only reads label/selectedLabel/value).
- Producers: module approvals (Approve/Reject), sender approvals
(Allow/Deny), channel approvals (Connect/Reject), and OneCLI credential
cards (Approve/Reject) annotate primary/danger. Multi-choice picker
options stay unstyled — a list of equals.
Purely additive: options without style render exactly as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hosted OneCLI gateway (api.onecli.sh) sends a structured summary field
on ApprovalRequest — { action, details: [{label, value}] } — that the SDK's
TypeScript type doesn't declare yet. When present, render it as the approval
card body (*Action:* plus labeled fields, fencing multi-line values) instead
of the raw METHOD host/path + bodyPreview, so approvers see what the agent
is doing (e.g. To / Subject / Body of an email send) rather than an HTTP
trace.
Rendering is defensive: non-string values are coerced via JSON.stringify (a
render throw would turn into a deny via handleRequest's catch), each value is
capped at 900 chars, and a 2600-char running budget keeps the card under
Slack's 3000-char section block limit — overflow is reported with an
explicit "…N field(s) omitted for length" line instead of a failed
delivery. Without a summary, the old bodyPreview fallback remains, now
capped and with the method/host/path as a footnote.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
extractAttachmentFiles (the channel-inbound attachment path) hardened only
the per-message inbox subdir, not the `inbox` root itself. A compromised
container can write inside its own session dir, so it could replace `inbox`
with a symlink: mkdirSync({recursive}) then followed it, and the realpath
containment check passed because it compared against realpathSync(inboxRoot)
— which had already followed the symlink. A brand-new attachment file (the
`wx` flag only blocks an existing dst) therefore landed outside the session
sandbox. This is the same symlink-follow class fixed for the A2A path in
#2828 (CWE-59), but reachable from ordinary inbound messages.
Extract the guard both inbound paths need into src/inbox-safety.ts
(ensureContainedInboxDir + isPathInside): lstat-reject a pre-placed symlink
or non-dir at the inbox root AND the per-message subdir before mkdir, then a
realpath containment check. forwardAttachedFiles and extractAttachmentFiles
now share it, removing the duplicated guard logic. Callers still write with
an exclusive flag (COPYFILE_EXCL / wx) and skip-with-warn on failure.
Adds src/session-manager.attachments.test.ts (red before this change, green
after) covering the symlinked inbox-root vector on the channel path.
forwardAttachedFiles hardened only the source side of A2A attachment
forwarding; the target side called fs.mkdirSync({recursive}) and
fs.copyFileSync without any symlink or containment checks. A compromised
target agent that can write inside its own session dir could pre-place
`inbox` (or `inbox/<future-msgId>`) as a symlink pointing anywhere
host-writable — mkdirSync silently follows it and copyFileSync lands
attacker-influenced bytes outside the sandbox (CWE-59, GHSA #2828). This
mirrors the existing defensive pattern in src/session-manager.ts
saveAttachments(): lstat-reject a pre-existing symlink/non-dir at the
inbox root and the per-message subdir before mkdir, realpath + isPathInside
containment check, and an exclusive (COPYFILE_EXCL) copy that refuses to
follow or overwrite a pre-placed symlinked destination. Failures log.warn
with structured context and skip rather than throw, so one bad attachment
never kills a batch. Tests cover a symlinked inbox dir, a symlinked
inbox/<msgId> subdir, a pre-existing symlinked destination file, and a
normal end-to-end forward regression.
Replace the terse "Approve delivery?" with a one-line legend that names all
three buttons and notes that "Reject with reason…" prompts the approver to
type a reason relayed back to the sender. The longer line also widens the
card bubble, easing the three-button-row truncation on platforms that size
buttons to the message width.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a third "Reject with reason…" button to module approval cards. Plain
Reject stays the instant fast path; the new option holds the row
(status='awaiting_reason'), DM-prompts the approver, and captures their
next DM (≤280 chars, truncated) as a one-line reason relayed to the
requesting agent as a single combined message. A ghosted hold is
finalized as a plain reject by the host sweep after ~5 min — restart-safe
via the durable DB row.
- Generalize the router message-interceptor to a list
(registerMessageInterceptor) so approvals can capture replies alongside
the permissions agent-naming flow.
- Share reject finalization across the instant, captured, and swept paths
via finalizeReject.
- Scope: all module approvals (create_agent, install_packages,
add_mcp_server); OneCLI credential cards are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review: move the assigned approver from the approval payload to a dedicated
`approver_user_id` column on pending_approvals.
- New migration adds the column; createPendingApproval + requestApproval write it.
- isAuthorizedApprovalClick reads approval.approver_user_id directly (drops the
payload-parsing helper); when set, only that exact user may resolve.
- The gate no longer stuffs `approver` into the payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review: drop the owner/global-admin override on assigned approvals. When an
approval names an approver, only that exact user can resolve it. (Non-assigned
approvals are unchanged — still group/owner authorized.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review: pull `approverUserId` into the `opts` destructure in requestApproval,
and `approver` out of `policy` in the gate, instead of accessing the property
twice. (policies.ts already binds args.* to locals.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove redundant doc/inline comments where the code speaks for itself; keep only
the non-obvious notes (return-vs-throw consume, ghost-gate cleanup, caller-does-
auth, reject-handled-elsewhere, stored-vs-click payload). Also drops a couple of
now-stale "target admin" descriptions. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review (no new pending_approvals column): the gate carries `approver` inside
the existing approval `payload`, and isAuthorizedApprovalClick authorizes the
named approver (or an owner/global admin) when an approval names one — reading
the real value at click time, no group re-derivation.
- `ncl policies set --approver` validates the user is an admin/owner of the
source OR target.
- Drops `approverAgentGroupId` and the agent_group_id stamp; `requestApproval`
keeps `approverUserId` only for delivery.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review: destructure the approval payload once instead of repeating
`payload.x`, and narrow `platform_id` up front so it's used directly (drops the
separate `targetAgentGroupId` local).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review, the policy approver is now required, not optional. Every policy
names one specific admin/owner of the target who approves.
- `approver` column is NOT NULL; `AgentMessagePolicy.approver` is non-nullable.
- `ncl policies set --approver <user-id>` is required and validated to be an
admin/owner of the target.
- The gate always delivers the card to `policy.approver`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review, add an optional `approver` to a policy: a specific admin/owner of
the target who receives the approval card (instead of all target admins). NULL
keeps the default (all target admins/owners).
- `approver` column on agent_message_policies; carried on AgentMessagePolicy.
- `ncl policies set --approver <user-id>` validates the user is an admin/owner
of the target at set-time, so the existing click-auth gate is unchanged.
- `requestApproval` gains `approverUserId` (single) to deliver the card to that
one user; the gate passes `policy.approver`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR review: extract `parseMessageContent` (text + attachment names from
the message content JSON) so `buildGateQuestion` reads as pure formatting, and
name the body-length cap (`GATE_CARD_BODY_MAX`) instead of a bare 1500.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR review: remove the `approvers` option entirely for v1 — the
approver is always the target group's admins/owners. Drops the `approvers`
DB column, the `--approvers` flag + its set-time validation, the now-unused
`approverUserIds` param on requestApproval, and the related tests. The
target-scoped approver pick (`approverAgentGroupId`) stays. Named approvers
can be re-added later via a migration when needed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR review: hoist session.agent_group_id into a named local
`sourceAgentGroupId`, mirroring `targetAgentGroupId`, and use it throughout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Shorten the verbose doc/inline comments added with the approval-policy gate
down to terse one-liners, matching the surrounding style (e.g. agent-destinations,
write-destinations). No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- policies.ts: drop the 10-line top banner. Sibling resource files carry no
descriptive header (only destinations.ts, and only for a non-obvious
side-effect); the prose already lives in the resource `description`.
- agent-message-policies.ts: remove `listMessagePolicies` — no production
caller (the `ncl policies list` op uses the generic table-based CRUD); only
its own test referenced it.
- message-gate.test.ts: assert the upsert-no-duplicate invariant via a direct
row count instead of the removed helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an optional, directed, per-message require-approval gate on top of an
existing agent-to-agent connection. No policy = today's free flow (fully
backward compatible). When a policy exists for A→B, each message A sends to B
is held, an approval card showing the message goes to B's admins, and the
message is delivered on approve / declined on reject. Rejecting one message
never blocks the connection.
- New `agent_message_policies` table (directed from→to; row exists = require
approval; `approvers` JSON, NULL = target admins). Deleted alongside its
connection so a stale rule can't reactivate on re-wire.
- Gate inside `routeAgentMessage` after the self/`hasDestination` checks:
holds the message via `requestApproval` and returns to consume it (like a
system action); the held message rides in the approval payload and is
re-routed by `applyA2aMessageGate` on approve. Self/internal messages are
never gated.
- `requestApproval` gains `approverAgentGroupId` / `approverUserIds` and stamps
`agent_group_id` on the pending row so the target's admins pass the
click-auth gate.
- `ncl policies list/set/remove`, operator-only (not in the container cli_scope
allowlist); `set` validates named approvers are admins/owners of the target.
Reuses the existing requestApproval / pending_approvals / approval-handler
spine (same shape as create_agent). Host-only; no container changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the agent provider a first-class, operator-chosen property instead of a
Claude-only assumption. Trunk gains the seams; the actual non-default payloads
(Codex first) install from the `providers` branch.
Setup
- A provider registry feeds a hard-wired setup picker (Claude | Codex). Picking
a non-default provider installs its payload (setup/add-codex.sh, channel-style),
runs a vault-only auth walkthrough (--step provider-auth), and records the pick
on the first agent before its first spawn.
- Picking Claude changes nothing — default installs are byte-for-byte unaffected.
Provider as a DB property
- Provider lives on container_configs.provider (materialized to container.json,
read by resolveProviderName). Creation stays provider-agnostic; the picked
provider is applied via the picked-provider seam. The deprecated
agent_groups.agent_provider path is not used.
Switching + memory
- Switch a live group with `ncl groups config update --provider` + restart.
- Memory never migrates at runtime — each provider keeps its own store. The
/migrate-memory skill carries a group's memory across a switch in either
direction (flat CLAUDE.local.md <-> memory/ scaffold). group-init seeds an
imported-agent-memory note for non-default providers; the runner's memory
definition reads it first turn. See docs/provider-migration.md.
No install-wide default, no runtime provider guard — switching is operator-by-
convention, consistent with the no-install-gating posture.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Modules can already register to handle an approval (registerApprovalHandler),
but nothing lets a module observe that an approval was resolved — e.g. to
clear an "awaiting approval" status indicator it set when the card went out.
Today that observation is only possible by core importing module code.
Add registerApprovalResolvedHandler/notifyApprovalResolved to the approvals
primitive and fire it at the three resolution exits in the response handler
(reject, approve-with-no-handler, approve-after-handler). Callback errors are
logged and isolated so one bad callback never blocks resolution or other
callbacks. The hook only fires for authorized clicks (it sits behind the
isAuthorizedApprovalClick gate) and carries the namespaced user id.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review-round fixes on the instance dimension:
- delivery/typing resolve adapters by exact registry key, never the
channelType fallback — a named instance with an offline adapter gets
offline handling, not a cross-identity send through a sibling bot;
the fallback scan (channelType-only callers) now warns when it
resolves through a differently-keyed instance
- migration runner only fails on FK violations a migration introduced:
pre-existing latent orphans (FK-OFF CLI surgery) are logged and
carried, not turned into a boot crash-loop
- typing re-trigger updates the full address (channelType, platformId,
threadId, instance) together — no torn entries on agent-shared
sessions spanning instances
- bridge rejects empty/whitespace instance names (URL-route and
state-namespace safety)
- add-github / add-linear SKILL.md wiring inserts include the NOT NULL
instance column
- drop the 10s same-platform boot stagger: operational policy, not
substrate — reintroducible skill-side for gateway-mode installs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Inbound: src/index.ts onInbound stamps `instance: adapter.instance ??
adapter.channelType` — the single host-side stamping seam; adapters stay
instance-blind and onInboundEvent (CLI) passes events through unchanged.
The router resolves the thread-policy adapter and the messaging group by
the receiving instance (exact-only — an unknown named instance auto-creates
its own group, persisting the instance, instead of hijacking a sibling's
row).
Outbound: ChannelDeliveryAdapter.deliver/setTyping grow a trailing
`instance` param (host-internal interface only — messages_out, destinations
and session_routing schemas are untouched; containers never see instance).
deliverMessage resolves the messaging group ORIGIN-SESSION-FIRST, so a
named instance's session replies through its own adapter even when a
sibling default row shares the same (channel_type, platform_id); dispatch
goes through getChannelAdapter(instance ?? channelType).
Typing: TypingTarget stores the instance and all three tick sites
(immediate, 4s interval, re-trigger) forward it, so the indicator fires
through the bot that owns the chat.
Also updates a raw-SQL fixture in groups.test.ts for the NOT NULL instance
column.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create_agent writes central-DB state (agent_groups, container_configs,
agent_destinations) and scaffolds host filesystem state, but the only
gate lived inside the untrusted container and is bypassed by writing the
outbound system row directly (the "host re-checks permission" comment was
false). Authorize host-side by CLI scope: trusted owner agent groups
(global scope) create sub-agents directly; confined groups require admin
approval via requestApproval. Adds regression tests for the branch.
Alternative to #2383 (which denies confined groups outright); co-authored
from that work.
Co-Authored-By: hinotoi-agent <paperlantern.agent@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Filter channel registration target options to the approver's authorized agent groups and re-check target authorization before applying a pending approval. Add regression coverage for scoped admins attempting to connect channels to out-of-scope groups.
The `destinations add` and `destinations remove` custom ops in the admin
CLI INSERT/DELETE rows in the central `agent_destinations` table, but
did not project the change into running sessions' `inbound.db`. The
agent-runner container reads its destination map from the per-session
projection, so until the next container spawn (`container-runner.ts`
hydrates on every wake), the running agent saw a stale map — explaining
the "dropped: unknown destination" symptom after a fresh `ncl
destinations add` even though the central row was clearly committed.
Same handler runs for both the direct-host path and the approval-execution
path because the `cli_command` approval handler in `dispatch.ts` re-enters
`dispatch()` as `caller: 'host'`, so the fix at the handler level covers
both surfaces.
Helper iterates over `getSessionsByAgentGroup(agentGroupId)` (every
active session for the affected agent), guarded by `hasTable('agent_destinations')`
and a lazy dynamic import of `writeDestinations` to keep the agent-to-agent
module optional. Per-session try/catch keeps one bad session from killing
the whole projection; failures are logged at WARN with session id + error.
Regression test invokes the dispatcher with `caller: 'host'` (the same
re-entry the approval handler uses after admin approves), with two active
sessions on the source agent group, and asserts the `destinations` row
lands in every session's inbound.db after `add` and is cleared after `remove`.
Fixes#2465
Platforms like Teams send userIds in "29:xxx" format which already
include a colon. Blindly prefixing with channelType produced double-
namespaced ids (e.g. "teams:29:xxx") that never matched the users
table, causing all approval clicks to be rejected. Mirror the
resolveOrCreateUser logic: only prefix when the raw id has no colon.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Decouple container restart from config updates — config CLI ops now only
write to the DB; restart is a separate `ncl groups restart` command with
--rebuild and --message flags. Add on_wake column to messages_in so wake
messages are only picked up by a fresh container's first poll, preventing
dying containers from stealing them during the SIGTERM grace window.
killContainer accepts an onExit callback for race-free respawn. Agent-
called restart auto-scopes to the calling session.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Source of truth for container runtime config moves from
groups/<folder>/container.json to a new container_configs table.
The file becomes a materialized view written at spawn time.
- New container_configs table with scalar columns (provider, model,
effort, image_tag, assistant_name, max_messages_per_prompt) and
JSON columns (mcp_servers, packages_apt, packages_npm, skills,
additional_mounts)
- Startup backfill seeds DB from existing container.json files
- materializeContainerJson() replaces readContainerConfig + ensureRuntimeFields
- Self-mod handlers (install_packages, add_mcp_server) write to DB
- Provider cascade simplified: session -> container_configs -> 'claude'
- ncl groups config-{get,update,add-mcp-server,remove-mcp-server,
add-package,remove-package} custom operations
- restartAgentGroupContainers() helper for config change propagation
- Container side unchanged (still reads /workspace/agent/container.json)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- host-core.test.ts: add in_reply_to: null to routeAgentMessage calls
(required after #2267 added the field to RoutableAgentMessage)
- agent-route.test.ts: use 'closed' instead of 'archived' (not a valid
Session status)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Squash merge of PR #2267 by ddaniels.
When an agent group has more than one active session, A2A replies landed
in the newest session via findSessionByAgentGroup's ORDER BY created_at
DESC. The session that asked the question never saw the answer.
Adds origin-aware return-path routing with three layers:
1. Direct return-path: if the reply has in_reply_to, look up the
triggering inbound row's source_session_id and route there.
2. Peer-affinity fallback: find the most recent A2A inbound from this
peer and use its source_session_id.
3. Legacy fallback: newest active session (pre-migration compat).
Container-side: MCP send_message/send_file now thread the current
batch's in_reply_to through to outbound rows via current-batch.ts.
Also flips our A2A bug-documenting test (#2332) from asserting the
broken behavior to asserting the fixed behavior.
Co-Authored-By: Doug Daniels <ddaniels888@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the hardcoded Approve/Ignore card with a multi-step flow:
- Single agent: "Connect to [name]" / "Connect new agent" / "Reject"
- Multiple agents: "Choose existing agent" (follow-up list) / "Connect new agent" / "Reject"
- "Connect new agent" prompts for a free-text name via DM, creates immediately on reply
- Add setMessageInterceptor router hook for capturing free-text replies
- Add resolveChannelName optional method to ChannelAdapter interface
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Filenames in forwardAttachedFiles arrived from the source agent's
messages_out content and were used directly in path.join on both
source outbox read and target inbox write. A value like `../evil.sh`
could escape `inbox/<a2a-id>/` on the target session (and similarly
the source outbox on read), breaking session isolation — an
adversarial or hallucinating sub-agent could overwrite files in
a sibling session.
Adds isSafeAttachmentName(name) — exported so it's unit-testable —
which rejects empty, `.`, `..`, anything containing `/`, `\`, or
NUL, and anything path.basename would strip. Guard runs before any
I/O. Unsafe names are dropped with a warning log, same pattern as
missing-source-file handling; a bad filename in one attachment
doesn't kill the whole route's text delivery.
Addresses Codex Review P1 on qwibitai/nanoclaw#1967.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Before: `send_file(to='parent')` from a sub-agent wrote the bytes to
the sub-agent's own session outbox, but agent-to-agent routing copied
only the content JSON — the target's inbound message referenced
`files: ['x.png']` but the bytes lived in a session directory the
target couldn't mount. Parent agents orchestrating sub-agents (e.g.
Design Team delegating illustration work to an Illustrator sub-agent
on Codex) received file-reference messages with nothing to forward.
Fix: on route, if the source's content has `files`, copy each referenced
file from `<source>/outbox/<src-msg-id>/` to
`<target>/inbox/<a2a-msg-id>/`, and emit `attachments` (the existing
formatter convention — see formatter.ts:223) with `localPath` relative
to `/workspace/`. The target formatter already renders these as
`[file: <name> — saved to /workspace/inbox/<a2a-id>/<name>]`, so the
target agent sees the path and can call `send_file(path=…, to=…)` to
forward onward.
Convention matches what session-manager.ts:256 already does for
base64-encoded channel-inbound attachments — same inbox layout, same
content shape. Nothing on the formatter/agent side needed to change.
## Scope
- `forwardAttachedFiles(source, target)` — pure-ish helper that copies
files and returns the attachments array.
- `forwardFileAttachments(msg, …)` — wraps the helper for the route
path: parses content, copies files if present, merges into any
existing `attachments`, re-serialises.
- `routeAgentMessage` — uses the rewritten content when writing the
target's inbound row.
- Log line now includes `forwardedFileCount` for observability.
Missing source files are skipped with a warning rather than killing
the route — a bad filename in a batch shouldn't drop the
accompanying text.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
getAskQuestionRender used to hardcode the card title and option labels
for pending_channel_approvals and pending_sender_approvals in the
DB-access layer, duplicating wording that already lived in the approval
modules. That caused a visible drift between the initial card title —
picked per event in channel-approval.ts ("📣 Bot mentioned in new chat"
vs. "💬 New direct message") — and the post-click render, which
always showed the constant "📣 Channel registration".
Mirror the pattern already used by pending_approvals: add title /
options_json columns on both pending_*_approvals tables via migration
013, have the approval modules write them at creation time, and let
getAskQuestionRender just SELECT.
- Migration 013 ALTERs the two tables to add title + options_json.
- PendingChannelApproval / PendingSenderApproval types and their
create functions grow the two fields.
- channel-approval.ts / sender-approval.ts normalize options once
and pass both title and options_json into the insert.
- getAskQuestionRender drops the hardcoded render objects and reads
the stored values.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moshe Krupper