Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Channel approval follow-up

- Channel approvals now provide private denial feedback and audit events; decision identities are stored as structured metadata; Mission Control can configure per-channel approver IDs. One-time decisions remain compatible without an allowlist, while standing grants require one.

# Changelog

All notable changes to this project will be documented in this file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const saveSchemas: Record<string, z.ZodType> = {
slack: z
.object({
enabled: z.boolean().optional(),
approverIds: z.array(z.string().trim().min(1).max(128)).max(50).optional(),
signingSecret: z.string().optional(),
botToken: z.string().optional(),
teamId: z.string().optional(),
Expand All @@ -93,13 +94,15 @@ const saveSchemas: Record<string, z.ZodType> = {
telegram: z
.object({
enabled: z.boolean().optional(),
approverIds: z.array(z.string().trim().min(1).max(128)).max(50).optional(),
botToken: z.string().optional(),
secretToken: z.string().optional(),
})
.strict(),
discord: z
.object({
enabled: z.boolean().optional(),
approverIds: z.array(z.string().trim().min(1).max(128)).max(50).optional(),
applicationId: z.string().optional(),
publicKey: z.string().optional(),
botToken: z.string().optional(),
Expand Down Expand Up @@ -171,7 +174,7 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
}

const configs = new ClawConnectorConfigService(ctx.tenantId);
await configs.save(ctx.channel, parsed.data as Record<string, string | boolean | undefined>, ctx.actor);
await configs.save(ctx.channel, parsed.data as Record<string, string | boolean | string[] | undefined>, ctx.actor);

// Routing is established after the save so the link always reflects what is
// actually stored. A failure here is reported, never fatal — the credentials
Expand Down
27 changes: 24 additions & 3 deletions apps/mission-control/components/connectors/channel-fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,16 @@ export const CHANNEL_FIELDS: Record<ChannelId, ChannelFieldConfig> = {
hint: 'Only needed when no bot token is set — otherwise it is read from Slack automatically.',
showWhileEmpty: 'botToken',
},
{
name: 'approverIds',
label: 'Authorized Approver IDs',
secret: false,
placeholder: 'U123, U456',
hint: 'Platform user IDs separated by commas. With no IDs, one-time approve/reject remains available to workspace members, while Always allow is blocked.',
},
],
testOverrideKeys: ['botToken'],
clears: 'signing secret and bot token',
clears: 'signing secret, bot token, and authorized approvers',
setupHint: 'Test Connection calls Slack’s auth.test with your bot token.',
},
telegram: {
Expand All @@ -73,9 +80,16 @@ export const CHANNEL_FIELDS: Record<ChannelId, ChannelFieldConfig> = {
placeholder: 'Generate or paste a high-entropy value',
hint: 'Sent by Telegram on every inbound request so we can verify it really came from Telegram.',
},
{
name: 'approverIds',
label: 'Authorized Approver IDs',
secret: false,
placeholder: 'U123, U456',
hint: 'Platform user IDs separated by commas. With no IDs, one-time approve/reject remains available to workspace members, while Always allow is blocked.',
},
],
testOverrideKeys: ['botToken'],
clears: 'bot token and secret token',
clears: 'bot token, secret token, and authorized approvers',
setupHint: 'Test Connection calls Telegram’s getMe with your bot token.',
},
discord: {
Expand Down Expand Up @@ -104,9 +118,16 @@ export const CHANNEL_FIELDS: Record<ChannelId, ChannelFieldConfig> = {
placeholder: 'Discord Developer Portal → Bot → Reset Token',
hint: 'Required to post replies — Discord has no long-lived reply mechanism besides bot-authenticated messages.',
},
{
name: 'approverIds',
label: 'Authorized Approver IDs',
secret: false,
placeholder: 'U123, U456',
hint: 'Platform user IDs separated by commas. With no IDs, one-time approve/reject remains available to workspace members, while Always allow is blocked.',
},
],
testOverrideKeys: ['botToken', 'applicationId', 'publicKey'],
clears: 'application id, public key, and bot token',
clears: 'application id, public key, bot token, and authorized approvers',
setupHint: 'Test Connection calls Discord’s /users/@me with your bot token. Register a /claw slash command and set the Interactions Endpoint URL to the webhook URL below.',
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ export function ConnectorSettingsForm({ channel }: { channel: ChannelId }) {
const handleSave = async () => {
setError(null);
try {
const result = await save.mutateAsync({ ...values, enabled: effectiveEnabled });
// The API stores an actual string[]; empty input explicitly clears the list.
const approverIds = values.approverIds === undefined
? {}
: { approverIds: Array.from(new Set(values.approverIds.split(/[,;\n]+/).map((id) => id.trim()).filter(Boolean))) };
const result = await save.mutateAsync({ ...values, ...approverIds, enabled: effectiveEnabled });
// Clear the inputs so the form goes back to showing masks — leaving the
// typed plaintext on screen after a successful save is needless exposure.
setValues({});
Expand Down
4 changes: 2 additions & 2 deletions apps/mission-control/hooks/use-connectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ async function fetchConnector(channel: ChannelId): Promise<ConnectorDetail> {

async function saveConnector(
channel: ChannelId,
input: Record<string, string | boolean | undefined>,
input: Record<string, string | boolean | string[] | undefined>,
): Promise<ConnectorSaveResult> {
const res = await fetch(`${BASE_PATH}/api/connectors/${channel}`, {
method: 'PUT',
Expand Down Expand Up @@ -116,7 +116,7 @@ export function useConnector(channel: ChannelId) {
export function useSaveConnector(channel: ChannelId) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: (input: Record<string, string | boolean | undefined>) => saveConnector(channel, input),
mutationFn: (input: Record<string, string | boolean | string[] | undefined>) => saveConnector(channel, input),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: connectorKeys.detail(channel) });
queryClient.invalidateQueries({ queryKey: connectorKeys.lists() });
Expand Down
7 changes: 4 additions & 3 deletions apps/workers/src/jobs/claw-gateway-run/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import { clawGatewayRunSchema } from './schema.js';
const log = createLogger('claw-gateway-run');

export async function handleClawGatewayRun(data: unknown): Promise<void> {
const { runId, tenantId, action, content, approvedBy } = clawGatewayRunSchema.parse(data);
const { runId, tenantId, action, content, actorId: newActorId, approvedBy } = clawGatewayRunSchema.parse(data);
const actorId = newActorId ?? approvedBy; // compatibility for pre-existing queued jobs

try {
const { executeRun, terminateRun, grantPendingToolsForRun } = await import('@chatbot/claw-studio');
Expand All @@ -39,7 +40,7 @@ export async function handleClawGatewayRun(data: unknown): Promise<void> {
runId,
deps,
kind: action === 'reject' ? 'reject' : 'cancel',
approvedBy: action === 'reject' ? approvedBy : undefined,
actorId: action === 'reject' ? actorId : undefined,
});
return;
}
Expand All @@ -48,7 +49,7 @@ export async function handleClawGatewayRun(data: unknown): Promise<void> {
await executeRun({
runId,
deps,
resume: action ? { action, content, approvedBy } : undefined,
resume: action ? { action, content, actorId } : undefined,
});
log.info({ runId, tenantId }, 'Gateway run handler finished');
} catch (error) {
Expand Down
2 changes: 2 additions & 0 deletions apps/workers/src/jobs/claw-gateway-run/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export const clawGatewayRunSchema = z.object({
/** The user's reply text, for clarification_response. */
content: z.string().optional(),
/** The platform user id that made this decision, when it came from a channel. */
actorId: z.string().optional(),
/** Accept already-queued jobs created before the actorId rename. */
approvedBy: z.string().optional(),
});

Expand Down
51 changes: 24 additions & 27 deletions docs/superpowers/claw-studio-known-issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,33 +78,30 @@ Fixed by `tokenizeName()` (maps `_`/`-` to spaces before matching) plus the two

---

## 🔴 11. Channel approval buttons are not authorized to a user

**Where:** `libs/claw-studio/src/gateway/gateway-service.ts` — the resume handler; and each
adapter's `parseInbound` for interactive payloads.

**Problem:** the resume path validates the channel signature, that the run exists, that it belongs to
the tenant the signature resolved to, and that the run's status matches the action. It captures the
clicker's platform user id (`message.userId`) and **never checks it against anything**. Any member of
the connected Slack workspace who can see an approval message can press Approve, Reject, or
"Always allow for this task".

**Verified:** traced the full resume path — no allowlist, no role check, no comparison of
`message.userId` against any stored value. `SlackConnectorConfig` has no allowlist field.
`ClawConnectorConfigService` stores none.

**Blast radius:** pre-existing for Approve/Reject, which grant a **one-time** action. `approve_always`
(added 2026-07-31) grants a **standing permission** on the task, so a single click by any workspace
member can let a scheduled task send mail or create issues unattended indefinitely. The feature did
not create this gap but it materially raises its consequence.

**Fix, roughly in order of value:**
1. Add an approver allowlist to each channel's connector config (platform user ids), checked in the
resume handler before enqueueing. A `whatsapp-contact-allowlist` design already exists in this
repo to follow.
2. Until then, consider gating `approve_always` to the Mission Control UI only (where the session is a
real authenticated studio login) and dropping the Slack button.
3. Record the approver on the run's `approval_decision` event so the action is at least attributable.
## 🟡 11. PARTIALLY FIXED (2026-10-19) — Channel approval identity

**Current behavior:** PR #65 added approverIds to each connector and checks the
platform actor before queuing approval actions. approve_always refuses requests
without a configured allowlist. Ordinary approve/reject without a list remain
backward-compatible under the maintainer's explicitly approved compromise.

**Follow-up from [PR #65 review](https://github.com/offloadmemory/agentfork/pull/65#pullrequestreview-5567988309):**
- Provide private, generic feedback for denied button presses.
- Audit denied actions with tenant, channel, actor ID, action, and reason.
- Rename the new-job identity field to actorId with compatibility for already-queued
payloads, and keep timeline content unchanged.
- Store the actor in structured approval_decision.metadata.
- Expose approverIds in the Mission Control connector configuration editor and
validate its values before saving.
- Test missing platform identities when an allowlist is configured.

**Compatibility:** One-time approve/reject without an allowlist is still allowed
until the maintainer explicitly decides to change that policy. Enabling
approve_always requires an explicit nonempty approver allowlist.

**Security scope:** Channel signatures authenticate workspaces, not individual
operators. Per-operator identities within Mission Control are tracked separately
in known issue #13.

---

Expand Down
14 changes: 14 additions & 0 deletions libs/claw-studio/src/connectors/adapters/discord.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,20 @@ describe('DiscordConnector', () => {
});
});

describe('sendDenial', () => {
it('returns an ephemeral interaction response visible only to the actor', async () => {
const req = signedRequest({
type: 3, application_id: APPLICATION_ID, data: { custom_id: 'x:y' },
});
const res = await new DiscordConnector().sendDenial(req, 'run-1');
expect(res.status).toBe(200);
expect(await res.json()).toMatchObject({
type: 4,
data: { flags: 64, content: expect.stringContaining('not authorized') },
});
});
});

describe('outbound', () => {
const run: ClawRunRecord = {
id: '1',
Expand Down
10 changes: 10 additions & 0 deletions libs/claw-studio/src/connectors/adapters/discord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,16 @@ export class DiscordConnector implements ChannelAdapter {
return json({ type: RESPONSE_DEFERRED_CHANNEL_MESSAGE });
}

/** Discord interaction response visible only to the clicking member. */
async sendDenial(req: Request, runId: string): Promise<Response> {
const parsed = await this.parse(req).catch(() => null);
if (parsed?.kind !== 'component') return this.sendAck(req, runId);
return json({
type: 4, // CHANNEL_MESSAGE_WITH_SOURCE
data: { content: 'You are not authorized to approve this request.', flags: 64 },
});
}

// ==========================================================================
// Outbound
// ==========================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ describe('SlackConnector inbound', () => {
expect(buttonAck.status).toBe(200);
await expect(buttonAck.text()).resolves.toBe('');
});

it('returns a private ephemeral denial without replacing Slack approval buttons', async () => {
const buttonBody = new URLSearchParams({
payload: JSON.stringify({ type: 'block_actions', team: { id: 'T123' }, actions: [] }),
}).toString();
const res = await new SlackConnector().sendDenial(slackRequest(buttonBody), 'run_1');
expect(res.status).toBe(200);
expect(await res.json()).toMatchObject({
response_type: 'ephemeral',
replace_original: false,
});
});

});

const SECRET_TOKEN = 'a-high-entropy-secret-token';
Expand Down Expand Up @@ -327,6 +340,26 @@ describe('TelegramConnector inbound', () => {
await expect(new TelegramConnector().parseInbound(req)).rejects.toThrow(/Not a user text message/);
});

it('answers a denied Telegram callback privately without posting to the chat', async () => {
const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(
new Response(JSON.stringify({ ok: true, result: true }), { status: 200 }),
);
try {
const req = telegramRequest({
callback_query: { id: 'cb1', data: 'claw:approve:run_xyz', from: { id: 42 } },
});
const res = await new TelegramConnector().sendDenial(req, 'run_xyz');
expect(res.status).toBe(200);
expect(fetchSpy).toHaveBeenCalledWith(
expect.stringContaining('/answerCallbackQuery'),
expect.objectContaining({ body: expect.stringContaining('not authorized') }),
);
expect(fetchSpy).not.toHaveBeenCalledWith(expect.stringContaining('/sendMessage'), expect.anything());
} finally {
fetchSpy.mockRestore();
}
});

it('keeps the approve callback payload inside Telegram\'s 64-byte limit', () => {
const runId = `run_${'x'.repeat(22)}`;
expect(Buffer.byteLength(`claw:approve:${runId}`)).toBeLessThanOrEqual(64);
Expand Down
14 changes: 14 additions & 0 deletions libs/claw-studio/src/connectors/adapters/slack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,20 @@ export class SlackConnector implements ChannelAdapter {
);
}

/** Ephemeral response to the clicking user; never modifies the shared approval message. */
async sendDenial(req: Request, runId: string): Promise<Response> {
const parsed = await this.parse(req).catch(() => null);
if (parsed?.kind !== 'block_actions') return this.sendAck(req, runId);
return new Response(
JSON.stringify({
response_type: 'ephemeral',
replace_original: false,
text: 'You are not authorized to approve this request.',
}),
{ status: 200, headers: { 'Content-Type': 'application/json' } },
);
}

// ==========================================================================
// Outbound
// ==========================================================================
Expand Down
17 changes: 17 additions & 0 deletions libs/claw-studio/src/connectors/adapters/telegram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,23 @@ export class TelegramConnector implements ChannelAdapter {
};
}

/** Answer the clicker's callback privately so denial does not post to the chat. */
async sendDenial(req: Request, runId: string): Promise<Response> {
try {
const { tenantId, update } = await this.parse(req);
if (tenantId && update?.callback_query?.id) {
await this.call(tenantId, 'answerCallbackQuery', {
callback_query_id: update.callback_query.id,
text: 'You are not authorized to approve this request.',
show_alert: true,
});
}
} catch (error) {
logger.warn({ error, runId }, 'Unable to acknowledge denied Telegram approval');
}
return new Response('', { status: 200 });
}

/**
* Telegram only needs an empty 200 — but a run started from a message gets a
* visible "on it" message whose id is remembered, so progress can be edited
Expand Down
17 changes: 17 additions & 0 deletions libs/claw-studio/src/connectors/config-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,23 @@ describe('ClawConnectorConfigService', () => {
});
});

it('round-trips, lists and clears approver IDs without encrypting public IDs', async () => {
const svc = new ClawConnectorConfigService('tenant-1');
await svc.save('telegram', { approverIds: ['42', '43', '42'] });
expect(await svc.getRaw('telegram')).toMatchObject({ approverIds: ['42', '43'] });
expect((await svc.getMasked('telegram')).fields.approverIds).toBe('42, 43');
await svc.save('telegram', { enabled: false });
expect(await svc.getRaw('telegram')).toMatchObject({ approverIds: ['42', '43'] });
await svc.save('telegram', { approverIds: [] });
expect(await svc.getRaw('telegram')).toMatchObject({ approverIds: [] });
});

it('rejects malformed approver IDs before saving', async () => {
const svc = new ClawConnectorConfigService('tenant-1');
await expect(svc.save('telegram', { approverIds: [''] })).rejects.toThrow('Invalid approverIds');
await expect(svc.save('telegram', { approverIds: Array(51).fill('1') })).rejects.toThrow('Invalid approverIds');
});

it('returns null from getRaw when the channel was never configured', async () => {
expect(await new ClawConnectorConfigService('tenant-1').getRaw('telegram')).toBeNull();
});
Expand Down
Loading
Loading