Skip to content
Open
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
8 changes: 7 additions & 1 deletion src/api/app-turn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,13 @@ export function createTurnMethods(

async listSessionApprovals(sessionId, viewer) {
if (!(await sessionForViewer(sessionId, viewer))) return [];
return pendingApprovalForSession(sessionId, { blockingOnly: false, viewer });
const children = (await deps.sessions.childrenOf?.(sessionId)) ?? [];
const lists = await Promise.all(
[sessionId, ...children.map((c) => c.id)].map((id) =>
pendingApprovalForSession(id, { blockingOnly: false, viewer }),
),
);
return lists.flat();
},

pendingApprovalForThread(threadRef, viewer) {
Expand Down
10 changes: 9 additions & 1 deletion src/api/artifact-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export interface ShareArtifactRequest {
email?: string;
permission?: Permission;
move?: boolean;
visibilityApproved?: true;
}

export type ShareArtifactResult =
Expand All @@ -97,7 +98,14 @@ export type ShareArtifactResult =
}
| {
ok: false;
code: "bad_request" | "not_found" | "forbidden" | "recipient_not_found" | "ambiguous_recipient" | "share_failed";
code:
| "bad_request"
| "not_found"
| "forbidden"
| "recipient_not_found"
| "ambiguous_recipient"
| "share_failed"
| "approval_required";
message: string;
candidates?: Array<{ id: string; label: string }>;
};
4 changes: 4 additions & 0 deletions src/api/control-service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { agentTurnCapability, VISIBILITY_APPROVAL_REQUIRED } from "../deploy/visibility-approval.ts";
import { unattendedGrantRefusal } from "../cron/authority.ts";
import { isDeepStrictEqual } from "node:util";
import type { Cron, CronFireLogEntry, CronSchedule, Destination, Principal, Webhook } from "../types.ts";
Expand Down Expand Up @@ -945,6 +946,9 @@ export function createControlService(app: App, scheduler?: Scheduler, admin?: Ad
};
}

if (isOrg && req.type === "deploy" && agentTurnCapability(capability) && req.visibilityApproved !== true)
return { ok: false, code: "approval_required", message: VISIBILITY_APPROVAL_REQUIRED };

try {
if (orgSkillCede) {
const promoted = await app.promoteSkill(home.id, toScope, capability.actorId, capability.liveActor === true);
Expand Down
12 changes: 12 additions & 0 deletions src/api/routes/deployments.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { agentTurnCapability, VISIBILITY_APPROVAL_REQUIRED } from "../../deploy/visibility-approval.ts";
import { EXTERNAL_APP_SHARING_OFF, externalAppSharingAllowed } from "../../feature-flags.ts";
import { SocksProxyAgent } from "socks-proxy-agent";
import { request as httpRequest } from "node:http";
Expand Down Expand Up @@ -1609,6 +1610,11 @@ export async function shareDeployment(ctx: ApiCtx): Promise<void> {
error: "bad_request",
message: "public access and person/scope access must be changed separately",
});
if (b.public && agentTurnCapability(capability)) {
const current = await app.getDeployment(params.id!);
if (current && current.public !== true)
return sendJson(res, 403, { error: "approval_required", message: VISIBILITY_APPROVAL_REQUIRED });
}
try {
const deployment = await app.setDeploymentPublic(params.id!, b.public, { createdBy: capability.actorId });
return sendJson(res, 200, {
Expand Down Expand Up @@ -1662,6 +1668,12 @@ export async function shareDeployment(ctx: ApiCtx): Promise<void> {
message: "more than one teammate matches",
candidates: target.candidates,
});
if (permission && parseScopeId(target.scope).kind === "org" && agentTurnCapability(capability)) {
const current = await app.getDeployment(params.id!);
const granted = current ? await app.deploymentGrantees(current.id) : [];
if (!granted.some((g) => g.scope === target.scope))
return sendJson(res, 403, { error: "approval_required", message: VISIBILITY_APPROVAL_REQUIRED });
}
try {
const invite =
typeof b.email === "string" && permission === "read"
Expand Down
25 changes: 23 additions & 2 deletions src/core/approval-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import type { PendingApprovalRecord } from "../types.ts";
import type { DurableMap } from "../persistence/durable-map.ts";
import type { DeliveryStore } from "../delivery/delivery-store.ts";
import { principalDestination } from "../reach/reach.ts";
import { samePerson } from "../directory/person.ts";
import { scopeId, type Destination } from "../types.ts";

export function approvalDeliveryKey(id: string, record: Pick<PendingApprovalRecord, "createdAt">): string {
return `command-approval:${id}:${record.createdAt ?? 0}`;
Expand All @@ -13,17 +15,36 @@ export function approvalDeliveryRecipient(actor: { externalId?: string } | undef
return id && !id.startsWith("system:") ? id : undefined;
}

export function approvalDestination(
request: Pick<
NonNullable<PendingApprovalRecord["request"]>,
"surface" | "triggerDestination" | "conversation" | "deliveryTarget"
>,
actorId: string,
): Destination {
const c = request.conversation;
if (request.surface === "slack" && c?.kind !== "dm" && c?.channelRef && request.deliveryTarget)
return {
type: "slack",
target: request.deliveryTarget,
audienceScopeId: scopeId(c.kind === "group" ? "group" : "channel", c.channelRef),
};
const d = request.surface === "cron" ? request.triggerDestination : undefined;
if (d && d.type !== "web" && (d.type !== "principal" || samePerson(d.target, actorId))) return d;
return principalDestination(actorId, actorId);
}

export function createApprovalStore(
backing: DurableMap<PendingApprovalRecord>,
deliveries: Pick<DeliveryStore, "enqueue">,
) {
async function deliver(id: string, record: PendingApprovalRecord): Promise<void> {
if (record.request?.surface !== "slack") return;
if (record.request?.surface !== "slack" && record.request?.surface !== "cron") return;
const actorId = approvalDeliveryRecipient(record.request.actor);
if (!actorId) return;
await deliveries.enqueue({
destination: {
...principalDestination(actorId, actorId),
...approvalDestination(record.request, actorId),
commandApprovalId: id,
...(record.request.slackSource
? { slackAccountId: record.request.slackSource.accountId, slackTeamId: record.request.slackSource.teamId }
Expand Down
22 changes: 22 additions & 0 deletions src/deploy/visibility-approval.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export type VisibilityWidening = "public" | "org";

export const VISIBILITY_APPROVAL_REQUIRED =
"making an app public or visible to the whole organization needs the owner's approval; use the apps tool so an approval card is posted";

export function visibilityApprovalCommand(app: string, widening: VisibilityWidening): string {
return widening === "public" ? `apps share ${app} public:true` : `apps share ${app} toScope:org`;
}

export function visibilityApprovalKey(app: string, widening: VisibilityWidening): string {
return `app-visibility:${JSON.stringify([app, widening])}`;
}

export function visibilityApprovalReason(app: string, widening: VisibilityWidening): string {
return widening === "public"
? `make app "${app}" public: anyone with the link can open it without signing in`
: `share app "${app}" with everyone in the organization`;
}

export function agentTurnCapability(claims: { sessionId?: string } | undefined): boolean {
return typeof claims?.sessionId === "string";
}
30 changes: 21 additions & 9 deletions src/harness/agent-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3300,7 +3300,13 @@ export function createAgentTools(ref: ToolContextRef, opts?: AgentToolsOptions):
text("[error] set public, email, or toScope, not more than one"),
true,
);
const d = await tc.setDeploymentPublic(id, params.public);
let d: Awaited<ReturnType<typeof tc.setDeploymentPublic>>;
try {
d = await tc.setDeploymentPublic(id, params.public);
} catch (e) {
if (e instanceof NeedsApproval) return blockOnApproval(callId, e, undefined, tool);
throw e;
}
return recordResult(
callId,
{ tool, action, type, id: d.id, public: d.public },
Expand All @@ -3314,14 +3320,20 @@ export function createAgentTools(ref: ToolContextRef, opts?: AgentToolsOptions):
text("[error] toScope or email is required unless public is set"),
true,
);
const r = await tc.shareArtifact({
type,
id,
...(params.toScope ? splitToScope(params.toScope) : {}),
...(params.email !== undefined ? { email: params.email } : {}),
...(params.permission !== undefined ? { permission: params.permission } : {}),
...(move ? { move: true } : {}),
});
let r: Awaited<ReturnType<typeof tc.shareArtifact>>;
try {
r = await tc.shareArtifact({
type,
id,
...(params.toScope ? splitToScope(params.toScope) : {}),
...(params.email !== undefined ? { email: params.email } : {}),
...(params.permission !== undefined ? { permission: params.permission } : {}),
...(move ? { move: true } : {}),
});
} catch (e) {
if (e instanceof NeedsApproval) return blockOnApproval(callId, e, undefined, tool);
throw e;
}
if (isUnavailable(r)) return unavailable(callId, tool);
if (!r.ok) {
const candidates = r.candidates?.length
Expand Down
46 changes: 30 additions & 16 deletions src/slack/deliveries.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { extractPrivateContinuation } from "./external-access.ts";
import { deployAccessMessage } from "./deploy-access.ts";
import { approvalDeliveryKey, approvalDeliveryRecipient } from "../core/approval-store.ts";
import { approvalDeliveryKey, approvalDeliveryRecipient, approvalDestination } from "../core/approval-store.ts";
import { samePerson } from "../directory/person.ts";
import { approvalMessage } from "./approval-cards.ts";
import { deliverKeychainCard } from "./keychain-approvals.ts";
Expand Down Expand Up @@ -29,7 +29,7 @@ import {
applyReactions,
} from "./lib.ts";
import type { SlackCoreClient } from "../api/slack-core-client.ts";
import type { Delivery } from "../types.ts";
import type { Delivery, PendingApprovalRecord } from "../types.ts";
import type { TurnFlow } from "./turn-flow.ts";
import { cleanAgentReplyForSlack, stripSlackDirectives } from "./messaging.ts";
import { cronIdOf } from "../sessions/session-store.ts";
Expand Down Expand Up @@ -241,6 +241,16 @@ export function createDeliveryPoller(deps: {
await deliverKeychainCard(core, client, d, channel, threadTs, deps.webUiPublicUrl);
return undefined;
}
if (d.destination.commandApprovalId) {
const approval = await approvalFor(core, d);
if (!approval) return undefined;
const card = approvalMessage([{ ...approval, reason: approval.reason ?? "Approval required" }]);
await postClient.chat.postMessage({
...slackReplyArgs(channel, card.text, threadTs),
blocks: card.blocks,
});
return undefined;
}
if (d.destination.react) {
const { messageTs, emoji } = d.destination.react;
const { failed } = await applyReactions(client, channel, messageTs, [emoji]);
Expand Down Expand Up @@ -432,20 +442,8 @@ export function createDeliveryPoller(deps: {
await deliverKeychainCard(core, client, d, dm, d.destination.threadTs, deps.webUiPublicUrl);
return undefined;
}
const commandApproval = d.destination.commandApprovalId
? await core.getApproval(d.destination.commandApprovalId)
: null;
const requester = approvalDeliveryRecipient(
commandApproval?.request?.actor as { externalId?: string } | undefined,
);
if (
d.destination.commandApprovalId &&
(!commandApproval ||
!requester ||
!samePerson(requester, d.destination.target) ||
d.idempotencyKey !== approvalDeliveryKey(d.destination.commandApprovalId, commandApproval))
)
return undefined;
const commandApproval = d.destination.commandApprovalId ? await approvalFor(core, d) : null;
if (d.destination.commandApprovalId && !commandApproval) return undefined;
let card: { text: string; blocks: Array<Record<string, unknown>> } | null = null;
if (d.destination.deploymentAccess) card = deployAccessMessage(d.destination.deploymentAccess, d.text);
if (commandApproval)
Expand Down Expand Up @@ -536,3 +534,19 @@ export function createDeliveryPoller(deps: {

return { pollDeliveries };
}

async function approvalFor(
core: { getApproval(id: string): Promise<unknown> },
d: { idempotencyKey?: string; destination: { type: string; target: string; commandApprovalId?: string } },
): Promise<(PendingApprovalRecord & { requestId: string }) | null> {
const id = d.destination.commandApprovalId!;
const approval = (await core.getApproval(id)) as (PendingApprovalRecord & { requestId: string }) | null;
const requester = approvalDeliveryRecipient(approval?.request?.actor as { externalId?: string } | undefined);
if (!approval?.request || !requester || d.idempotencyKey !== approvalDeliveryKey(id, approval)) return null;
const expected = approvalDestination(approval.request, requester);
const matches =
d.destination.type === "principal"
? samePerson(requester, d.destination.target)
: expected.type === d.destination.type && expected.target === d.destination.target;
return matches ? approval : null;
}
39 changes: 35 additions & 4 deletions src/tools/primitives.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import {
visibilityApprovalCommand,
visibilityApprovalKey,
visibilityApprovalReason,
type VisibilityWidening,
} from "../deploy/visibility-approval.ts";
import type { MemoryCaptureMetadata } from "../memory/records.ts";
import { disclosedMemory } from "../memory/disclosure.ts";
import { MaskedExecutionError, executionSecretEnv, createExactSecretValueMasker } from "../security/secret-masking.ts";
Expand Down Expand Up @@ -585,6 +591,16 @@ export function createToolContext(deps: ToolContextDeps): ToolContext {
);
};

const requireVisibilityApproval = (app: string, widening: VisibilityWidening): void => {
const command = visibilityApprovalCommand(app, widening);
const approvalKey = visibilityApprovalKey(app, widening);
if (deps.authorizeCommand(command, approvalKey, true)) return;
throw new NeedsApproval(command, visibilityApprovalReason(app, widening), "approval", undefined, approvalKey, {
session: false,
always: false,
});
};

const ledger = deps.ledger ?? createNullLedger();
const runId = deps.runId;
const attempt = deps.attempt ?? 1;
Expand Down Expand Up @@ -1100,6 +1116,10 @@ export function createToolContext(deps: ToolContextDeps): ToolContext {
},

async setDeploymentPublic(id: string, isPublic: boolean) {
if (isPublic) {
const current = await deps.deploy.getDeployment(id);
if (current && current.public !== true) requireVisibilityApproval(current.name ?? current.id, "public");
}
const d = await deps.deploy.setDeploymentPublic(id, isPublic, { createdBy: deps.createdBy });
return { id: d.id, ...(d.name ? { name: d.name } : {}), public: d.public === true };
},
Expand Down Expand Up @@ -1328,11 +1348,22 @@ export function createToolContext(deps: ToolContextDeps): ToolContext {
async (c, cl) => c.writeSoul(content, cl, expectedVersion),
(r) => r.ok,
),
shareArtifact: (req) =>
controlOp(
(c, cl) => c.shareArtifact(req, cl),
shareArtifact: async (req) => {
let approved = false;
const targetScope = req.scope === "org" && orgScopeId ? orgScopeId : req.scope;
if (req.type === "deploy" && targetScope !== undefined && parseScopeId(targetScope).kind === "org") {
const current = await deps.deploy.getDeployment(req.id);
const granted = current && orgScopeId ? await deps.deploy.deploymentGrantees(current.id) : [];
if (req.move || !granted.some((g) => g.scope === orgScopeId)) {
requireVisibilityApproval(current?.name ?? req.id, "org");
approved = true;
}
}
return controlOp(
(c, cl) => c.shareArtifact(approved ? { ...req, visibilityApproved: true } : req, cl),
(r) => r.ok,
),
);
},

post: (text, opts, files) =>
surfaceOp(
Expand Down
2 changes: 1 addition & 1 deletion src/triggers/run-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ export async function runTrigger(deps: TriggerDeps, spec: TriggerSpec): Promise<
sessionId = res.sessionId;
if (res.status === "silent") return;
if (res.status === "pending_approval") {
note = "hit a require_approval command — failed closed (no human at fire/event time)";
note = "held a command for the owner's approval; it did not run";
console.warn(`[trigger] ${spec.surface} ${spec.fireKey} ${note}`);
return;
}
Expand Down
32 changes: 32 additions & 0 deletions test/agent-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4088,6 +4088,38 @@ test("background process guidance reflects the configured sandbox token lifetime
assert.doesNotMatch(unlimited, /turn tokens expire \d+ hours/);
});

test("apps visibility widening pauses the turn on an approval card instead of applying", async () => {
const held = (key: string) =>
new NeedsApproval(`apps share x ${key}`, "widen", "approval", undefined, key, { session: false, always: false });
const ref: ToolContextRef = {
current: {
...fakeToolContext(),
async setDeploymentPublic() {
throw held("public-share");
},
async shareArtifact() {
throw held("org");
},
},
pendingApprovals: [],
};
const apps = createAgentTools(ref, { controlTools: true }).find((t) => t.name === "apps")!;
for (const args of [
{ action: "share", id: "x", public: true },
{ action: "share", id: "x", toScope: "org" },
]) {
assert.match(textOut(await call(apps, args)), /needs human approval/);
}
assert.equal(ref.pausedOnApproval, true);
assert.deepEqual(
ref.pendingApprovals!.map((a) => [a.approvalKey, a.grantModes]),
[
["public-share", { session: false, always: false }],
["org", { session: false, always: false }],
],
);
});

test("every tool schema carries the optional retrySafe flag, which is stripped before the tool runs and recorded on the call", async () => {
const emitted: Emitted[] = [];
const ref: ToolContextRef = {
Expand Down
Loading
Loading