Skip to content

Commit 711b79e

Browse files
committed
fix(dashboard-agent): apply the token's cap as a second ceiling, and finalise only this turn's messages
Review of #4418: the cloud path builds the ability from the user's role, so a read-only delegated token could exchange it for a write JWT; and the finalisable set was the whole replayed transcript rather than what the turn produced.
1 parent 2ff7ec0 commit 711b79e

7 files changed

Lines changed: 63 additions & 37 deletions

File tree

apps/webapp/app/services/userActorEnvironment.server.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
*/
1010

1111
import { json } from "@remix-run/server-runtime";
12-
import { type RbacAbility, scopesWithinAbility, type UserActorClaims } from "@trigger.dev/rbac";
12+
import {
13+
buildJwtAbility,
14+
type RbacAbility,
15+
scopesWithinAbility,
16+
type UserActorClaims,
17+
} from "@trigger.dev/rbac";
1318
import { $replica } from "~/db.server";
1419

1520
export const FORBIDDEN_ENVIRONMENT_CODE = "forbidden_environment";
@@ -112,20 +117,28 @@ export async function resolveUserActorEnvironmentScope(
112117
/** Mirrors the RBAC fallback's own default. */
113118
const CAPLESS_USER_ACTOR_SCOPES = ["read:all"];
114119

115-
/** A delegated token must never mint something more capable than itself, so it is the ceiling. */
120+
/**
121+
* A delegated token must never mint something more capable than itself. Two ceilings apply:
122+
* the actor's own ability (their role) and the token's `cap`. The role alone is not enough —
123+
* a read-only agent token belongs to a user who may well be allowed to write.
124+
*/
116125
export function clampUserActorScopes(
117126
requestedScopes: string[] | undefined,
118127
userActor: UserActorClaims,
119128
ability: RbacAbility
120129
): { scopes: string[]; deniedScopes: string[] } {
121-
const requested =
122-
requestedScopes && requestedScopes.length > 0
123-
? requestedScopes
124-
: (userActor.cap ?? CAPLESS_USER_ACTOR_SCOPES);
130+
const cap = userActor.cap ?? CAPLESS_USER_ACTOR_SCOPES;
131+
const requested = requestedScopes && requestedScopes.length > 0 ? requestedScopes : cap;
125132

126-
const { deniedScopes } = scopesWithinAbility(requested, ability);
133+
const denied = new Set([
134+
...scopesWithinAbility(requested, ability).deniedScopes,
135+
...scopesWithinAbility(requested, buildJwtAbility(cap)).deniedScopes,
136+
]);
127137

128-
return { scopes: requested.filter((scope) => !deniedScopes.includes(scope)), deniedScopes };
138+
return {
139+
scopes: requested.filter((scope) => !denied.has(scope)),
140+
deniedScopes: [...denied],
141+
};
129142
}
130143

131144
function assertClaimIsOptional(userActor: UserActorClaims): void {

apps/webapp/test/dashboardAgentDelegatedScopeCeiling.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,18 @@ it("still hands over the reads the cap does carry", async () => {
5858
expect(clamped.scopes).toEqual(["read:runs"]);
5959
});
6060

61+
it("refuses a write the cap forbids even when the user's role allows it", async () => {
62+
// The cloud path builds the ability from the user's role, not from the token's cap —
63+
// so the role alone would hand a read-only agent token a write JWT.
64+
const { claims } = await abilityFor(AGENT_CAP);
65+
const roleAllowsEverything = { can: () => true, canSuper: () => false } as never;
66+
67+
const clamped = clampUserActorScopes(["write:runs"], claims, roleAllowsEverything);
68+
69+
expect(clamped.scopes).toEqual([]);
70+
expect(clamped.deniedScopes).toContain("write:runs");
71+
});
72+
6173
it("keeps a capless delegated token read-only", async () => {
6274
const { ability, claims } = await abilityFor();
6375

apps/webapp/test/dashboardAgentTranscriptStore.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ describe("invariant 3: an ordinary transcript write can never change a stored me
311311
await persistTurn(agentDb, {
312312
chatId,
313313
messages: [toolMessage("a1", "output-available")],
314+
finalizeMessageIds: ["a1"],
314315
session: { publicAccessToken: "pat_store" },
315316
});
316317

@@ -645,6 +646,8 @@ describe("a write can no longer lose a message another process appended", () =>
645646
await persistTurn(agentDb, {
646647
chatId,
647648
messages: [{ ...(card.message as Record<string, unknown>), tampered: true }],
649+
// Even named outright, a durable event is not this turn's to rewrite.
650+
finalizeMessageIds: [cardId],
648651
session: { publicAccessToken: "pat_store" },
649652
});
650653
const afterCard = (await rows(prisma, chatId)).find(

internal-packages/dashboard-agent-db/drizzle/meta/0002_snapshot.json

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,14 @@
7474
"compositePrimaryKeys": {
7575
"chat_messages_chat_id_message_id_pk": {
7676
"name": "chat_messages_chat_id_message_id_pk",
77-
"columns": [
78-
"chat_id",
79-
"message_id"
80-
]
77+
"columns": ["chat_id", "message_id"]
8178
}
8279
},
8380
"uniqueConstraints": {
8481
"chat_messages_chat_position_key": {
8582
"name": "chat_messages_chat_position_key",
8683
"nullsNotDistinct": false,
87-
"columns": [
88-
"chat_id",
89-
"position"
90-
]
84+
"columns": ["chat_id", "position"]
9185
}
9286
},
9387
"policies": {},
@@ -408,10 +402,7 @@
408402
"compositePrimaryKeys": {
409403
"chat_turn_evals_chat_id_turn_pk": {
410404
"name": "chat_turn_evals_chat_id_turn_pk",
411-
"columns": [
412-
"chat_id",
413-
"turn"
414-
]
405+
"columns": ["chat_id", "turn"]
415406
}
416407
},
417408
"uniqueConstraints": {},
@@ -690,10 +681,7 @@
690681
"compositePrimaryKeys": {
691682
"watch_batches_environment_id_cadence_minutes_pk": {
692683
"name": "watch_batches_environment_id_cadence_minutes_pk",
693-
"columns": [
694-
"environment_id",
695-
"cadence_minutes"
696-
]
684+
"columns": ["environment_id", "cadence_minutes"]
697685
}
698686
},
699687
"uniqueConstraints": {},
@@ -846,10 +834,7 @@
846834
"compositePrimaryKeys": {
847835
"watch_submissions_chat_id_client_request_id_pk": {
848836
"name": "watch_submissions_chat_id_client_request_id_pk",
849-
"columns": [
850-
"chat_id",
851-
"client_request_id"
852-
]
837+
"columns": ["chat_id", "client_request_id"]
853838
}
854839
},
855840
"uniqueConstraints": {},
@@ -1298,4 +1283,4 @@
12981283
"schemas": {},
12991284
"tables": {}
13001285
}
1301-
}
1286+
}

internal-packages/dashboard-agent-db/drizzle/meta/_journal.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,4 @@
2424
"breakpoints": true
2525
}
2626
]
27-
}
27+
}

internal-packages/dashboard-agent-db/src/queries.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,12 @@ export async function persistTurn(
571571
runId?: string | null;
572572
};
573573
settlements?: PendingInvestigationSettlement[];
574+
/**
575+
* The ids this turn produced. Only these may be rewritten in place — `messages` is the
576+
* whole replayed transcript, so finalising all of it would let a later turn overwrite a
577+
* durable event that happens to be in the agent's history.
578+
*/
579+
finalizeMessageIds?: string[];
574580
}
575581
): Promise<PersistTurnResult> {
576582
return db.transaction(async (tx) => {
@@ -608,14 +614,15 @@ export async function persistTurn(
608614
);
609615
const messages = [...params.messages, ...cards.filter((card) => !existing.has(card.id))];
610616

611-
// `onTurnStart` stores the turn's messages mid-flight, so the completed bodies arrive
612-
// here against ids that already exist: without finalisation the transcript would keep
613-
// the half-finished tool call the user never saw the end of. Settlement cards are
614-
// durable events and stay insert-only.
617+
// `onTurnStart` stores this turn's messages mid-flight, so their completed bodies arrive
618+
// here against ids that already exist: without finalisation the transcript would keep the
619+
// half-finished tool call the user never saw the end of. Everything else — earlier turns,
620+
// settlement cards, host-appended wakes — stays insert-only.
621+
// A settlement card is never the turn's to rewrite, however it was named.
615622
const finalizable = new Set(
616-
params.messages
617-
.map((message) => messageIdOf(params.chatId, message))
618-
.filter((id) => !id.startsWith(`${INVESTIGATION_SETTLEMENT_MESSAGE_ID_PREFIX}:`))
623+
(params.finalizeMessageIds ?? []).filter(
624+
(id) => !id.startsWith(`${INVESTIGATION_SETTLEMENT_MESSAGE_ID_PREFIX}:`)
625+
)
619626
);
620627

621628
await storeChatMessages(tx, { chatId: params.chatId, messages, finalizable });

internal-packages/dashboard-agent/src/dashboard-agent.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,9 +456,15 @@ export const dashboardAgent = chat.agent({
456456
// and closing cards of whatever was left running. Settling a row on a separate
457457
// operation is what could leave a terminal row whose card never arrived — and the
458458
// stale sweep only selects `in_progress`, so nothing would ever repair it.
459+
// Only what this turn produced may be finalised; the rest of the snapshot is history.
460+
const produced = [...(newMessages ?? []), ...(responseMessage ? [responseMessage] : [])]
461+
.map((message) => (message as { id?: unknown }).id)
462+
.filter((id): id is string => typeof id === "string");
463+
459464
const { settled } = await store.persistTurn({
460465
chatId,
461466
messages: mergeMessagesById(uiMessages, failure ? [failure] : []),
467+
finalizeMessageIds: [...produced, ...(failure ? [failure.id] : [])],
462468
session: {
463469
publicAccessToken: chatAccessToken,
464470
lastEventId,

0 commit comments

Comments
 (0)