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: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
request for an administrator to switch on a work app, grant it to a Bot, or connect a Shared app's
team account is kept, with who answered it and when. It starts empty and nothing is backfilled.

### A Bot's Slack or Teams link code works only in a direct message

The `link <code>` message a Bot's Reaching you shows now links only when it is sent to the app in a
direct message. Sent in a channel, with or without a mention, it is cancelled and the sender is told
to make a new one, as an account code from Connections already was. Whoever sends the code first is
linked to that Bot: their messages run it as its owner, and its questions and approval requests go
to them, so a code others have seen must not still work.

### A Bot with an app or web research can use its browser in a conversation again

A Bot that held any app or research tool, such as Parallel Search, could not open a page, click or
Expand Down
2 changes: 1 addition & 1 deletion app/src/components/delivery/reaching-you.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ function ConnectDialog({
<Item size="sm" variant="muted">
<ItemContent>
<ItemDescription className="line-clamp-none">
Send this message to the OpenBot app in{" "}
Send this message to the OpenBot app in a direct message in{" "}
{linked.platform === "teams" ? "Microsoft Teams" : "Slack"}{" "}
within {Math.ceil((linked.expiresAt - Date.now()) / 60_000)}{" "}
minutes. It links that account to this conversation and Bot.
Expand Down
3 changes: 2 additions & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,8 @@ Slack app of its own.

Point OpenTag's `AGENT_URL` at `<public URL>/api/delivery/webhooks/opentag/agent`. A person links
their Slack or Teams identity from a Bot's page, under **Reaching you**: OpenBot shows a one-time `link <code>` message,
valid for ten minutes, which they send to the OpenTag app. After that, only that person's messages
valid for ten minutes, which they send to the OpenTag app in a direct message. A code posted in a
shared channel is cancelled, because whoever sends it first is linked to that Bot. After that, only that person's messages
reach their chosen Bot, in their own OpenBot conversation, with learning, governance, approvals and
audit on every turn. Anyone unlinked is told how to link and nothing runs. Approval requests appear
as OpenTag approval cards (allow once, always allow, cancel) and are decided through the approvals
Expand Down
54 changes: 37 additions & 17 deletions server/src/delivery/opentag-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ const LINK =
const LINKS = new RegExp(LINK.source, "gi");

/**
* A channel post the app was not mentioned in: everyone there has seen any account code in it, so
* each one is cancelled, quietly (an observe run never replies). The codes are redacted rather than
* A channel post the app was not mentioned in: everyone there has seen any link code in it,
* Reachability or account, so each one is cancelled, quietly (an observe run never replies). The codes are redacted rather than
* the message dropped, so the post still fires the owners' message triggers without carrying a
* code. A failed cancel is logged and never stops the triggers.
*/
Expand All @@ -128,22 +128,36 @@ async function withoutLinkCodes(
) {
const codes = [...text.matchAll(LINKS)].map((m) => m[1]?.toLowerCase());
if (!codes.length) return text;
if (sender.transport === "slack" && deps.identity)
for (const code of codes) {
if (!code) continue;
try {
for (const code of codes) {
if (!code) continue;
try {
await cancelReachabilityCode(deps, code);
if (sender.transport === "slack" && deps.identity)
await deps.identity.discard(code);
} catch (error) {
console.error(
JSON.stringify({
type: "opentag-link-code-discard-error",
errorType: error instanceof Error ? error.name : "UnknownError",
}),
);
}
} catch (error) {
console.error(
JSON.stringify({
type: "opentag-link-code-discard-error",
errorType: error instanceof Error ? error.name : "UnknownError",
}),
);
}
}
return text.replace(LINKS, "link [code removed]");
}

/** Spends a live Reachability code without binding anyone; true when one was cancelled. */
async function cancelReachabilityCode(deps: OpenTagAgentDeps, code: string) {
const challenge = await deps.store.readChallenge(code);
return Boolean(
challenge &&
(await deps.store.consumeChallenge(code, challenge.ownerUserId)),
);
}

const CANCELLED_IN_SHARED =
"That code was posted where others could see it, so I cancelled it. Create a new one in OpenBot and send it to me in a direct message.";

const platformName = (transport: ChatTransport) =>
transport === "teams" ? "Microsoft Teams" : "Slack";

Expand Down Expand Up @@ -408,16 +422,22 @@ async function link(
code: string,
) {
const challenge = await deps.store.readChallenge(code);
// A Reachability code binds whoever sends it to its issuer's Bot: their messages run as the
// issuer, and the Bot's questions and approval requests go to them. Anyone else in a shared
// conversation has seen it and could send it first, so it is only redeemed in a direct message
// and one posted here is spent, the same rule as for an account code below.
if (challenge && !sender.private) {
await deps.store.consumeChallenge(code, challenge.ownerUserId);
return { text: CANCELLED_IN_SHARED };
}
if (!challenge && sender.transport === "slack" && deps.identity) {
// Anyone else in a shared conversation has now seen the code and could send it first, binding
// their own Slack account to its issuer, so an account code is only redeemed in a direct
// message and one posted here is cancelled (an unmentioned channel post is cancelled in
// `withoutLinkCodes`).
if (!sender.private) {
if (await deps.identity.discard(code))
return {
text: "That code was posted where others could see it, so I cancelled it. Create a new one in OpenBot and send it to me in a direct message.",
};
return { text: CANCELLED_IN_SHARED };
return {
text: "That link code expired, was already used, or is for another app. Create a new one in OpenBot.",
};
Expand Down
60 changes: 53 additions & 7 deletions server/tests/delivery-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function fixture(
) {
const calls: unknown[] = [];
const decisions: unknown[] = [];
const consumed: unknown[] = [];
const twilio = createTwilioTransport({
accountSid: "AC1",
authToken: "token",
Expand Down Expand Up @@ -83,7 +84,10 @@ function fixture(
address: null,
}
: null,
consumeChallenge: async () => true,
consumeChallenge: async (...input) => {
consumed.push(input);
return true;
},
bind: async (input) => {
calls.push({ bind: input });
return { ...bound, ...input };
Expand Down Expand Up @@ -139,7 +143,7 @@ function fixture(
inbox: async () => ({ questions: [] }),
},
});
return { routes, calls, decisions };
return { routes, calls, decisions, consumed };
}
const sender = (
user = "slack:T1:U1",
Expand Down Expand Up @@ -322,15 +326,41 @@ test("an identity code in a direct message is never discarded", async () => {
await run(f, { context: [sender()], ...linkMessage(IDENTITY_CODE) });
expect(discarded).toEqual([]);
});
test("a Reachability code still links from a shared conversation", async () => {
const { f, redeemed, discarded } = identityFixture("linked");
const REACHABILITY_CODE = "11111111-2222-4333-8444-555555555555";
test("a Reachability code sent where others can read it is cancelled, never bound", async () => {
// Whoever sends a Reachability code is bound to its issuer's Bot and decides its approvals as
// the issuer, so a code others have seen is spent, exactly like an account code.
for (const conversation of [{ id: "C0123456789", kind: "channel" }, null]) {
const { f, redeemed, discarded } = identityFixture("linked");
const result = await run(f, {
context: [sender("slack:T1:U1", conversation)],
...linkMessage(REACHABILITY_CODE),
});
expect(said(result.events)).toBe(CANCELLED);
expect(f.consumed).toEqual([[REACHABILITY_CODE, "owner"]]);
expect(f.calls).toEqual([]);
expect(redeemed).toEqual([]);
expect(discarded).toEqual([]);
}
});
test("a Reachability code is cancelled from a shared conversation without an account adapter", async () => {
const f = fixture({ bound: false });
const result = await run(f, {
context: [sender("slack:T1:U1", { id: "C0123456789", kind: "channel" })],
...linkMessage("11111111-2222-4333-8444-555555555555"),
...linkMessage(REACHABILITY_CODE),
});
expect(said(result.events)).toBe(CANCELLED);
expect(f.consumed).toEqual([[REACHABILITY_CODE, "owner"]]);
expect(f.calls).toEqual([]);
});
test("a Reachability code in a direct message still links", async () => {
const { f } = identityFixture("linked");
const result = await run(f, {
context: [sender()],
...linkMessage(REACHABILITY_CODE),
});
expect(said(result.events)).toContain("Linked.");
expect(redeemed).toEqual([]);
expect(discarded).toEqual([]);
expect(f.calls).toHaveLength(1);
});
const observedMessage = (content: string) => ({
context: [
Expand Down Expand Up @@ -368,6 +398,22 @@ test("an identity code pasted in a channel without a mention is cancelled quietl
);
}
});
test("a Reachability code pasted in a channel without a mention is cancelled quietly and kept out of triggers", async () => {
const { f, redeemed, discarded } = identityFixture("linked");
const result = await run(
f,
observedMessage(`here: link ${REACHABILITY_CODE.toUpperCase()}`),
);
expect(said(result.events)).toBe("");
expect(f.consumed).toEqual([[REACHABILITY_CODE, "owner"]]);
expect(redeemed).toEqual([]);
expect(discarded).toEqual([REACHABILITY_CODE]);
const triggers = triggersOf(f);
expect(triggers).toHaveLength(1);
expect(JSON.stringify(triggers[0]).toLowerCase()).not.toContain(
REACHABILITY_CODE,
);
});
test("an ordinary observed channel message reaches triggers unchanged and discards nothing", async () => {
const { f, discarded } = identityFixture("linked");
const result = await run(f, observedMessage("deploy is broken"));
Expand Down
Loading