diff --git a/src/api/app-turn.ts b/src/api/app-turn.ts index ac60943fe5..3892a2c49a 100644 --- a/src/api/app-turn.ts +++ b/src/api/app-turn.ts @@ -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) { diff --git a/src/api/artifact-share.ts b/src/api/artifact-share.ts index fc8e322a95..b22e989b4e 100644 --- a/src/api/artifact-share.ts +++ b/src/api/artifact-share.ts @@ -83,6 +83,7 @@ export interface ShareArtifactRequest { email?: string; permission?: Permission; move?: boolean; + visibilityApproved?: true; } export type ShareArtifactResult = @@ -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 }>; }; diff --git a/src/api/control-service.ts b/src/api/control-service.ts index 28a36a4a7e..8920cb5580 100644 --- a/src/api/control-service.ts +++ b/src/api/control-service.ts @@ -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"; @@ -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); diff --git a/src/api/routes/deployments.ts b/src/api/routes/deployments.ts index 1e80adc845..63bcb3bab9 100644 --- a/src/api/routes/deployments.ts +++ b/src/api/routes/deployments.ts @@ -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"; @@ -1609,6 +1610,11 @@ export async function shareDeployment(ctx: ApiCtx): Promise { 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, { @@ -1662,6 +1668,12 @@ export async function shareDeployment(ctx: ApiCtx): Promise { 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" diff --git a/src/core/approval-store.ts b/src/core/approval-store.ts index ab27420928..102a955d03 100644 --- a/src/core/approval-store.ts +++ b/src/core/approval-store.ts @@ -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): string { return `command-approval:${id}:${record.createdAt ?? 0}`; @@ -13,17 +15,36 @@ export function approvalDeliveryRecipient(actor: { externalId?: string } | undef return id && !id.startsWith("system:") ? id : undefined; } +export function approvalDestination( + request: Pick< + NonNullable, + "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, deliveries: Pick, ) { async function deliver(id: string, record: PendingApprovalRecord): Promise { - 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 } diff --git a/src/deploy/visibility-approval.ts b/src/deploy/visibility-approval.ts new file mode 100644 index 0000000000..0c5cd4e9b9 --- /dev/null +++ b/src/deploy/visibility-approval.ts @@ -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"; +} diff --git a/src/harness/agent-tools.ts b/src/harness/agent-tools.ts index ea4ec2e317..75b60104d6 100644 --- a/src/harness/agent-tools.ts +++ b/src/harness/agent-tools.ts @@ -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>; + 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 }, @@ -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>; + 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 diff --git a/src/slack/deliveries.ts b/src/slack/deliveries.ts index e154ae98d0..d53ef1039a 100644 --- a/src/slack/deliveries.ts +++ b/src/slack/deliveries.ts @@ -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"; @@ -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"; @@ -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]); @@ -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> } | null = null; if (d.destination.deploymentAccess) card = deployAccessMessage(d.destination.deploymentAccess, d.text); if (commandApproval) @@ -536,3 +534,19 @@ export function createDeliveryPoller(deps: { return { pollDeliveries }; } + +async function approvalFor( + core: { getApproval(id: string): Promise }, + 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; +} diff --git a/src/tools/primitives.ts b/src/tools/primitives.ts index 08b6df1d31..2957d1b6a6 100644 --- a/src/tools/primitives.ts +++ b/src/tools/primitives.ts @@ -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"; @@ -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; @@ -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 }; }, @@ -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( diff --git a/src/triggers/run-trigger.ts b/src/triggers/run-trigger.ts index 8451709b69..7d070adbbe 100644 --- a/src/triggers/run-trigger.ts +++ b/src/triggers/run-trigger.ts @@ -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; } diff --git a/test/agent-tools.test.ts b/test/agent-tools.test.ts index fa3b0a4047..6e873d1ac0 100644 --- a/test/agent-tools.test.ts +++ b/test/agent-tools.test.ts @@ -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 = { diff --git a/test/approval-store.test.ts b/test/approval-store.test.ts index 46cf3b0d92..34844b224a 100644 --- a/test/approval-store.test.ts +++ b/test/approval-store.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { createApprovalStore } from "../src/core/approval-store.ts"; +import { approvalDestination, createApprovalStore } from "../src/core/approval-store.ts"; import { createMemoryMap } from "../src/persistence/durable-map.ts"; import { createDeliveryStore } from "../src/delivery/delivery-store.ts"; import type { PendingApprovalRecord } from "../src/types.ts"; @@ -92,3 +92,47 @@ test("system approvals stay pending without enqueuing or reviving an impossible assert.deepEqual(await deliveries.pending("principal"), []); assert.equal((await deliveries.get(stale.id))?.expiredAt, expiredAt, "a sweep must not revive the stale DM"); }); + +test("scheduled-task approvals post at the task's own destination, like credential cards", async () => { + const channel = { type: "slack", target: "C9:100.200", audienceScopeId: "channel:C9" as const }; + const deliveries = createDeliveryStore(); + const approvals = createApprovalStore(createMemoryMap(), deliveries); + const toChannel = record("cron:K1"); + toChannel.request!.surface = "cron"; + toChannel.request!.triggerDestination = channel; + await approvals.put("A1", toChannel); + const [channelCard] = await deliveries.pending("slack"); + assert.equal(channelCard?.destination.target, "C9:100.200"); + assert.equal(channelCard?.destination.commandApprovalId, "A1"); + + const noDestination = record("cron:K2"); + noDestination.request!.surface = "cron"; + await approvals.put("A2", noDestination); + const ownerCards = await deliveries.pending("principal"); + assert.equal(ownerCards.length, 1, "no destination falls back to the owner's DM"); + assert.equal(ownerCards[0]!.destination.target, "U1"); + + const otherPerson = record("cron:K3"); + otherPerson.request!.surface = "cron"; + otherPerson.request!.triggerDestination = { type: "principal", target: "U2", audienceScopeId: "personal:U2" }; + assert.equal(approvalDestination(otherPerson.request!, "U1").target, "U1", "never DMs someone else's card"); + + const web = record("cron:K4"); + web.request!.surface = "cron"; + web.request!.triggerDestination = { type: "web", target: "s1" } as never; + assert.equal(approvalDestination(web.request!, "U1").target, "U1"); +}); + +test("Slack channel approvals post in the requesting thread; DMs stay in the DM", async () => { + const thread = record("slack:C1:1.0"); + thread.request!.conversation = { kind: "channel", threadRef: "slack:C1:1.0", channelRef: "C1" }; + thread.request!.deliveryTarget = "C1:1.0"; + assert.deepEqual(approvalDestination(thread.request!, "U1"), { + type: "slack", + target: "C1:1.0", + audienceScopeId: "channel:C1", + }); + const dm = record("dm:D1"); + dm.request!.deliveryTarget = "D1"; + assert.equal(approvalDestination(dm.request!, "U1").type, "principal"); +}); diff --git a/test/live-slack/scenarios-visibility-approval.ts b/test/live-slack/scenarios-visibility-approval.ts new file mode 100644 index 0000000000..25e280a77f --- /dev/null +++ b/test/live-slack/scenarios-visibility-approval.ts @@ -0,0 +1,144 @@ +import { createHmac } from "node:crypto"; +import { assert, type Actor, type Ctx, type Env, type Scenario } from "./harness.ts"; +import { sleep, type SlackMessage } from "./slack.ts"; + +const EVENTS_URL = () => + process.env.SLACK_EVENTS_TARGET_URL ?? `http://127.0.0.1:${process.env.SLACK_EVENTS_PORT ?? "8182"}/slack/events`; + +async function waitFor(label: string, fn: () => Promise, timeoutMs = 360_000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const v = await fn().catch(() => undefined); + if (v !== undefined) return v; + await sleep(3000); + } + throw new Error(`timed out waiting for ${label}`); +} + +const buttons = (m: SlackMessage): Array<{ action_id: string; value: string; block_id?: string }> => { + const block = (m.blocks ?? []).find((b) => b.type === "actions") as + { block_id?: string; elements?: Array<{ action_id: string; value: string }> } | undefined; + return (block?.elements ?? []).map((e) => ({ ...e, block_id: block?.block_id })); +}; + +const isCard = (env: Env, app: string) => (m: SlackMessage) => + m.user === env.botUserId && + buttons(m).some((b) => b.action_id === "hilo_allow_once") && + (JSON.stringify(m.blocks ?? []) + (m.text ?? "")).includes(app); + +async function click(env: Env, clicker: Actor, channel: string, card: SlackMessage): Promise { + const action = buttons(card).find((a) => a.action_id === "hilo_allow_once")!; + const body = `payload=${encodeURIComponent( + JSON.stringify({ + type: "block_actions", + team: { id: env.teamId, domain: "e2e" }, + user: { id: clicker.userId, username: clicker.handle, team_id: env.teamId }, + api_app_id: "AE2E", + container: { + type: "message", + message_ts: card.ts, + channel_id: channel, + is_ephemeral: false, + ...(card.thread_ts ? { thread_ts: card.thread_ts } : {}), + }, + channel: { id: channel, name: "c" }, + message: { type: "message", ts: card.ts, text: card.text, user: env.botUserId, thread_ts: card.thread_ts }, + actions: [{ ...action, type: "button", action_ts: `${Date.now() / 1000}` }], + }), + )}`; + const ts = String(Math.floor(Date.now() / 1000)); + const signature = `v0=${createHmac("sha256", process.env.SLACK_SIGNING_SECRET ?? "") + .update(`v0:${ts}:${body}`) + .digest("hex")}`; + const res = await fetch(EVENTS_URL(), { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + "X-Slack-Request-Timestamp": ts, + "X-Slack-Signature": signature, + }, + body, + signal: AbortSignal.timeout(15_000), + }); + assert.equal(res.status, 200); +} + +async function messages(client: Actor["client"], channel: string, root?: string): Promise { + return root + ? [...(await client.replies(channel, root)), ...(await client.history(channel, root))] + : client.history(channel, "0"); +} + +const appPrompt = (name: string) => + `Publish a tiny app named "${name}": a Node server.js that answers "ok" on $PORT, entrypoint "node server.js". Then make it public so anyone with the link can open it. Once it is actually public, post a message containing exactly VIS_DONE and nothing else; never write that token otherwise.`; + +const MODES = ["thread", "dm", "subagent", "cron"] as const; +const ACTOR = { thread: "alice", dm: "bob", subagent: "carol", cron: "alice" } as const; + +export const visibilityApprovalScenarios: Scenario[] = MODES.map((mode) => ({ + name: `visibility-approval-card-${mode}`, + lane: "parallel", + tags: ["twin", "apps"], + actors: [ACTOR[mode]], + timeoutMs: 20 * 60_000, + async run(ctx: Ctx) { + const env = ctx.env; + const owner = ctx.actor(ACTOR[mode]); + const name = `vis-${mode}-${env.runId.slice(-6)}`; + const ownerDm = await owner.client.openDm(env.botUserId); + const since = String(Date.now() / 1000); + let channel: string; + let root: string | undefined; + if (mode === "dm") { + channel = ownerDm; + await owner.client.post(ownerDm, appPrompt(name)); + } else { + const ch = await ctx.freshChannel(); + channel = ch.id; + const as = ch.as(owner); + if (mode === "thread") root = await as.mention(appPrompt(name)); + else if (mode === "subagent") + root = await as.mention( + `Open a sub-agent session (sessions tool, action open) and have IT do this work, not you: ${appPrompt(name)}`, + ); + else { + root = await as.mention( + `Publish a tiny app named "${name}" (Node server.js answering "ok" on $PORT, entrypoint "node server.js") and keep it private. Then schedule a one-shot cron for 1 minute from now that posts back to this thread and, when it fires, makes "${name}" public and then posts a message containing exactly VIS_DONE and nothing else (never write that token otherwise). Reply SCHEDULED now.`, + ); + } + } + const card = await waitFor("approval card where the request was made", async () => + (await messages(owner.client, channel, root)).find((m) => Number(m.ts) > Number(since) && isCard(env, name)(m)), + ).catch(async (error) => { + const all = (await messages(owner.client, channel, root).catch(() => [])) + .filter((m) => m.user === env.botUserId) + .map((m) => `${JSON.stringify((m.text ?? "").slice(0, 60))}${buttons(m).length ? "[buttons]" : ""}`); + (error as Error).message += ` (bot messages: ${all.join(" | ") || "none"})`; + const inDm = mode !== "dm" && (await owner.client.history(ownerDm, since)).some(isCard(env, name)); + throw new Error(`${(error as Error).message}${inDm ? " (it went to the owner's DM instead)" : ""}`); + }); + const problems: string[] = []; + if (!/public|anyone with the link/i.test(JSON.stringify(card.blocks ?? []) + (card.text ?? ""))) + problems.push("card does not say it makes the app public"); + if (mode !== "dm" && (await owner.client.history(ownerDm, since)).some(isCard(env, name))) + problems.push("card also went to the owner's DM"); + const early = (await messages(owner.client, channel, root)).some( + (m) => m.user === env.botUserId && Number(m.ts) > Number(since) && (m.text ?? "").trim() === "VIS_DONE", + ); + if (early) problems.push("app was reported public before approval"); + await click(env, owner, channel, card); + await waitFor("work to resume after approval", async () => + (await messages(owner.client, channel, root)).some( + (m) => m.user === env.botUserId && Number(m.ts) >= Number(card.ts) && (m.text ?? "").trim() === "VIS_DONE", + ) + ? true + : undefined, + ).catch(async () => { + const seen = (await messages(owner.client, channel, root).catch(() => [])) + .filter((m) => m.user === env.botUserId && Number(m.ts) >= Number(card.ts)) + .map((m) => JSON.stringify((m.text ?? "").slice(0, 80))); + problems.push(`approving did not resume the work (bot after card: ${seen.join(" | ") || "nothing"})`); + }); + assert.deepEqual(problems, [], problems.join("; ")); + }, +})); diff --git a/test/live-slack/scenarios.ts b/test/live-slack/scenarios.ts index c2c4aae034..f2603ada5b 100644 --- a/test/live-slack/scenarios.ts +++ b/test/live-slack/scenarios.ts @@ -7,6 +7,7 @@ import { multiUserScenarios } from "./scenarios-multiuser.ts"; import { twinScenarios } from "./scenarios-twin.ts"; import { deployAccessScenarios } from "./scenarios-deploy-access.ts"; import { credentialApprovalScenarios } from "./scenarios-credential-approval.ts"; +import { visibilityApprovalScenarios } from "./scenarios-visibility-approval.ts"; const RAW_MARKDOWN_ARTIFACTS: Array<[string, RegExp]> = [ ["**bold**", /\*\*[^*\n]+\*\*/], @@ -550,4 +551,5 @@ export const scenarios: Scenario[] = [ ...twinScenarios, ...deployAccessScenarios, ...credentialApprovalScenarios, + ...visibilityApprovalScenarios, ]; diff --git a/test/session-metadata-authz.test.ts b/test/session-metadata-authz.test.ts index 7fd2884800..902e4a3928 100644 --- a/test/session-metadata-authz.test.ts +++ b/test/session-metadata-authz.test.ts @@ -110,3 +110,18 @@ test("a dropped project member cannot discard or retitle a session they can no l assert.ok(await built.sessions.get(sessionId), "the session survives the refused discard"); assert.equal(await built.app.discardSession(sessionId, "owner"), true, "the owner can still discard it"); }); + +test("a sub-agent's held approval is listed inline in its parent session too", async () => { + const built = freshApp(); + const parent = await built.app.turn(dm("plan the work", "web:alice:parent", "alice")); + const child = await built.app.turn(dm("!run git push --force origin main", "web:alice:child", "alice")); + assert.equal(child.status, "pending_approval"); + await built.sessions.setParentSession(child.sessionId!, parent.sessionId!); + const inParent = await built.app.listSessionApprovals(parent.sessionId!, "alice"); + assert.deepEqual( + inParent.map((a) => a.requestId), + (await built.app.listSessionApprovals(child.sessionId!, "alice")).map((a) => a.requestId), + ); + assert.equal(inParent.length, 1); + assert.deepEqual(await built.app.listSessionApprovals(parent.sessionId!, "carol"), [], "outsiders still see nothing"); +}); diff --git a/test/share-deployment.test.ts b/test/share-deployment.test.ts index a67d9b86e9..ce332a124f 100644 --- a/test/share-deployment.test.ts +++ b/test/share-deployment.test.ts @@ -27,6 +27,8 @@ import type { CapabilityClaims } from "../src/auth/capability-token.ts"; import type { RecipientResolution } from "../src/directory/directory-store.ts"; import type { Sandbox, SandboxHandle } from "../src/sandbox/sandbox.ts"; import { scopeId } from "../src/types.ts"; +import { NeedsApproval } from "../src/tools/primitives.ts"; +import { visibilityApprovalKey } from "../src/deploy/visibility-approval.ts"; import type { FeatureFlagStore } from "../src/feature-flags.ts"; const externalSharingOn = { enabled: async () => true } as unknown as FeatureFlagStore; @@ -401,7 +403,8 @@ const appSandbox = (): Sandbox => { } as unknown as Sandbox; }; -function toolCtx(deploy: DeployService): ToolContext { +function toolCtx(deploy: DeployService, approved: string[] = []): ToolContext { + const approvals = new Set(approved); return createToolContext({ sandbox: appSandbox(), provision: async () => ({}) as SandboxHandle, @@ -410,7 +413,7 @@ function toolCtx(deploy: DeployService): ToolContext { { scopeId: scopeId("org", "default-org"), mountPath: "global", mode: "ro" }, ], commandPolicy: () => ({}) as never, - authorizeCommand: () => false, + authorizeCommand: (_command: string, key?: string) => key !== undefined && approvals.delete(key), grantedHandles: [], workspace: {} as never, deploy, @@ -888,3 +891,57 @@ test("uniform app share uses the same invitation sender", async () => { assert.equal(result.invitation?.emailSent, true); assert.deepEqual(sent, ["invitee@example.com"]); }); + +test("widening an app to public or org-wide holds for approval; narrowing never does", async () => { + const { deploy } = makeDeploy(); + await toolCtx(deploy).publish({ entrypoint: "x", name: "held" }); + + await assert.rejects( + () => toolCtx(deploy).setDeploymentPublic("held", true), + (e: unknown) => + e instanceof NeedsApproval && + e.approvalKey === visibilityApprovalKey("held", "public") && + e.grantModes?.always === false && + e.grantModes.session === false, + ); + assert.equal((await deploy.getDeployment("held"))?.public, undefined, "held request did not widen"); + + const made = await toolCtx(deploy, [visibilityApprovalKey("held", "public")]).setDeploymentPublic("held", true); + assert.equal(made.public, true, "an approved request applies"); + assert.equal((await toolCtx(deploy).setDeploymentPublic("held", true)).public, true, "already public needs nothing"); + assert.equal((await toolCtx(deploy).setDeploymentPublic("held", false)).public, false, "narrowing needs no approval"); + await assert.rejects( + () => toolCtx(deploy).setDeploymentPublic("held", true), + NeedsApproval, + "an approval is single-use: widening again asks again", + ); +}); + +test("agent turn tokens cannot widen an app through the HTTP share endpoint", async () => { + const { app, deploy } = apiHarness(); + await deploy.deploy({ + ownerScopeId: scopeId("personal", "U1"), + createdBy: "U1", + entrypoint: "x", + files: [], + name: "api-held", + }); + const agent = { ...cap("U1"), sessionId: "s1" } as CapabilityClaims; + + const pub = await callShare(app, agent, "api-held", { public: true }); + assert.equal(pub.status, 403); + assert.equal(pub.body.error, "approval_required"); + assert.equal((await deploy.getDeployment("api-held"))?.public, undefined); + + const org = await callShare(app, agent, "api-held", { scope: "org" }); + assert.equal(org.status, 403); + assert.equal(org.body.error, "approval_required"); + assert.equal((await deploy.reachDeployment("api-held", "U2")).status, "denied"); + + assert.equal((await callShare(app, agent, "api-held", { public: false })).status, 200, "narrowing is allowed"); + assert.equal( + (await callShare(app, cap("U1"), "api-held", { public: true })).status, + 200, + "the owner's own UI action is the approval", + ); +}); diff --git a/test/share-verb.test.ts b/test/share-verb.test.ts index 2a4bcbb7e0..2523c5803f 100644 --- a/test/share-verb.test.ts +++ b/test/share-verb.test.ts @@ -631,3 +631,27 @@ test("SkillStore.promote supersedes an existing same-name org record instead of const orgRecords = (await store.list()).filter((x) => x.scopeId === scopeId("org", ORG) && x.status === "published"); assert.equal(orgRecords.length, 1); }); + +test("an agent turn cannot share or move an app org-wide without an approved request", async () => { + for (const move of [false, true]) { + const state = ownerState(); + const svc = createControlService(fakeApp(state)); + const agent = { ...cap("U1"), sessionId: "s1" }; + const held = await svc.shareArtifact({ type: "deploy", id: "D1", scope: "org", ...(move ? { move } : {}) }, agent); + assert.equal(held.ok, false); + assert.equal(!held.ok && held.code, "approval_required"); + assert.equal((await grantsOf(state)).length, 0); + assert.equal(state.moves.length, 0); + const approved = await svc.shareArtifact( + { type: "deploy", id: "D1", scope: "org", visibilityApproved: true, ...(move ? { move } : {}) }, + agent, + ); + assert.ok(approved.ok, JSON.stringify(approved)); + } + const svc = createControlService(fakeApp(ownerState())); + const channel = await svc.shareArtifact( + { type: "deploy", id: "D1", scope: scopeId("channel", "C1") }, + { ...cap("U1"), sessionId: "s1" }, + ); + assert.ok(channel.ok, "narrower grants are unaffected"); +}); diff --git a/test/slack-deliveries.test.ts b/test/slack-deliveries.test.ts index e02383cc16..13fab299b3 100644 --- a/test/slack-deliveries.test.ts +++ b/test/slack-deliveries.test.ts @@ -385,3 +385,25 @@ for (const type of ["slack", "group", "principal"]) { assert.deepEqual(recovered.acknowledgements, ["D1"]); }); } + +test("a scheduled task's approval card renders at the task's destination, and only there", async () => { + const at = { type: "slack", target: "C1:100.200" }; + const approval = { + requestId: "A1", + command: "apps share x public:true", + reason: "approval", + grantModes: { session: false, always: false }, + request: { surface: "cron", actor: { externalId: "U1" }, triggerDestination: at }, + }; + const out = await deliver({ commandApprovalId: "A1" }, undefined, undefined, "Approval needed", { approval }); + assert.equal(out.posts.length, 1); + assert.match(JSON.stringify(out.posts[0]!.blocks), /hilo_allow_once/); + const elsewhere = await deliver( + { target: "C2:1.2", commandApprovalId: "A1" }, + undefined, + undefined, + "Approval needed", + { approval }, + ); + assert.equal(elsewhere.posts.length, 0, "a card never lands in a channel the task does not post to"); +}); diff --git a/test/surface-spine-routing.test.ts b/test/surface-spine-routing.test.ts index d3d5573ed1..e2b0493010 100644 --- a/test/surface-spine-routing.test.ts +++ b/test/surface-spine-routing.test.ts @@ -637,7 +637,17 @@ for (const command of ["!finish-silent-approval", "!finish-silent-paused"]) { const result = await built.app.turn({ ...mention(command, "C-approval", command), async: false }); assert.equal(result.status, "pending_approval"); assert.equal(result.pendingApprovals?.length, 1); - assert.equal((await built.deliveries.pending("slack")).length, 0); + const pending = (await built.deliveries.pending("slack")) as Array<{ + destination: { target: string; commandApprovalId?: string }; + }>; + assert.equal(pending.filter((d) => !d.destination.commandApprovalId).length, 0, "silence posts no reply"); + const cards = pending.filter((d) => d.destination.commandApprovalId); + assert.equal(cards.length, 1, "the approval card is the only Slack delivery"); + assert.equal( + cards[0]!.destination.target, + `slack:C-approval:${command}`, + "the card lands in the requesting thread", + ); }); }