mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-22 22:51:15 +00:00
fix(workboard): avoid timing leaks in claim authorization (#104259)
* test(workboard): cover claim token lifecycle auth * fix(workboard): compare claim tokens safely Co-authored-by: 0668001099 <yang.yujie1@xydigit.com> * refactor(workboard): centralize claim identity checks --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { safeEqualSecret } from "openclaw/plugin-sdk/security-runtime";
|
||||
import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";
|
||||
import {
|
||||
BLOCKED_TOO_LONG_MS,
|
||||
@@ -347,10 +348,9 @@ export function assertCanMutateClaimedCard(
|
||||
}
|
||||
const ownerId = normalizeOptionalString(scope.ownerId);
|
||||
const token = normalizeOptionalString(scope.token);
|
||||
if (claim.ownerId === ownerId || (token && claim.token === token)) {
|
||||
return;
|
||||
if (claim.ownerId !== ownerId && !safeEqualSecret(token, claim.token)) {
|
||||
throw new Error(`card is claimed by ${claim.ownerId}.`);
|
||||
}
|
||||
throw new Error(`card is claimed by ${claim.ownerId}.`);
|
||||
}
|
||||
|
||||
export function retryBudgetExhausted(card: WorkboardCard): boolean {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { isDeepStrictEqual } from "node:util";
|
||||
import { isFutureDateTimestampMs } from "openclaw/plugin-sdk/number-runtime";
|
||||
import { safeEqualSecret } from "openclaw/plugin-sdk/security-runtime";
|
||||
import {
|
||||
appendEvent,
|
||||
assertCanMutateClaimedCard,
|
||||
@@ -51,10 +52,22 @@ import { WorkboardPromoteStore } from "./store-promote.js";
|
||||
import type {
|
||||
WorkboardArtifact,
|
||||
WorkboardCard,
|
||||
WorkboardClaim,
|
||||
WorkboardNotification,
|
||||
WorkboardRunAttempt,
|
||||
} from "./types.js";
|
||||
|
||||
function assertClaimIdentity(claim: WorkboardClaim, input: WorkboardHeartbeatInput): void {
|
||||
const token = normalizeOptionalString(input.token);
|
||||
const ownerId = normalizeOptionalString(input.ownerId);
|
||||
if (token && !safeEqualSecret(token, claim.token)) {
|
||||
throw new Error("claim token does not match.");
|
||||
}
|
||||
if (!token && ownerId && ownerId !== claim.ownerId) {
|
||||
throw new Error("claim owner does not match.");
|
||||
}
|
||||
}
|
||||
|
||||
export class WorkboardWorkflowStore extends WorkboardPromoteStore {
|
||||
async claim(
|
||||
id: string,
|
||||
@@ -140,14 +153,7 @@ export class WorkboardWorkflowStore extends WorkboardPromoteStore {
|
||||
throw new Error("card is not claimed.");
|
||||
}
|
||||
const now = Math.max(Date.now(), claim.lastHeartbeatAt + 1);
|
||||
const token = normalizeOptionalString(input.token);
|
||||
const ownerId = normalizeOptionalString(input.ownerId);
|
||||
if (token && token !== claim.token) {
|
||||
throw new Error("claim token does not match.");
|
||||
}
|
||||
if (!token && ownerId && ownerId !== claim.ownerId) {
|
||||
throw new Error("claim owner does not match.");
|
||||
}
|
||||
assertClaimIdentity(claim, input);
|
||||
const nextClaim = {
|
||||
...claim,
|
||||
lastHeartbeatAt: now,
|
||||
@@ -192,14 +198,7 @@ export class WorkboardWorkflowStore extends WorkboardPromoteStore {
|
||||
: normalizeStatus(input.status, existing.status);
|
||||
const claim = existing.metadata?.claim;
|
||||
if (claim) {
|
||||
const token = normalizeOptionalString(input.token);
|
||||
const ownerId = normalizeOptionalString(input.ownerId);
|
||||
if (token && token !== claim.token) {
|
||||
throw new Error("claim token does not match.");
|
||||
}
|
||||
if (!token && ownerId && ownerId !== claim.ownerId) {
|
||||
throw new Error("claim owner does not match.");
|
||||
}
|
||||
assertClaimIdentity(claim, input);
|
||||
}
|
||||
return await this.updateCard(
|
||||
id,
|
||||
|
||||
@@ -1038,6 +1038,25 @@ describe("WorkboardStore", () => {
|
||||
const released = await store.releaseClaim(card.id, { ownerId: "main", status: "review" });
|
||||
expect(released.status).toBe("review");
|
||||
expect(released.metadata?.claim).toBeUndefined();
|
||||
|
||||
const tokenCard = await store.create({ title: "Token-authorized worker", status: "todo" });
|
||||
const tokenClaim = await store.claim(tokenCard.id, { ownerId: "main", ttlSeconds: 60 });
|
||||
|
||||
await expect(
|
||||
store.heartbeat(tokenCard.id, { ownerId: "other", token: "wrong-token" }),
|
||||
).rejects.toThrow(/token does not match/);
|
||||
await expect(
|
||||
store.heartbeat(tokenCard.id, { ownerId: "other", token: tokenClaim.token }),
|
||||
).resolves.toMatchObject({ metadata: { claim: { ownerId: "main" } } });
|
||||
|
||||
await expect(
|
||||
store.releaseClaim(tokenCard.id, { ownerId: "other", token: "wrong-token" }),
|
||||
).rejects.toThrow(/token does not match/);
|
||||
const tokenReleased = await store.releaseClaim(tokenCard.id, {
|
||||
ownerId: "other",
|
||||
token: tokenClaim.token,
|
||||
});
|
||||
expect(tokenReleased.metadata?.claim).toBeUndefined();
|
||||
});
|
||||
|
||||
it("atomically guards and adopts dispatcher workspace authority", async () => {
|
||||
@@ -1912,6 +1931,29 @@ describe("WorkboardStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("checks matching claim tokens inside queued card writes", async () => {
|
||||
const store = new WorkboardStore(createMemoryStore());
|
||||
const card = await store.create({ title: "Token-scoped mutation" });
|
||||
await store.claim(card.id, { ownerId: "main", token: "test-auth-token" });
|
||||
|
||||
await expect(
|
||||
store.addComment(
|
||||
card.id,
|
||||
{ body: "rejected write" },
|
||||
{ ownerId: "other", token: "test-token-placeholder" },
|
||||
),
|
||||
).rejects.toThrow(/claimed by main/);
|
||||
await expect(
|
||||
store.addComment(
|
||||
card.id,
|
||||
{ body: "accepted write" },
|
||||
{ ownerId: "other", token: "test-auth-token" },
|
||||
),
|
||||
).resolves.toMatchObject({
|
||||
metadata: { comments: [expect.objectContaining({ body: "accepted write" })] },
|
||||
});
|
||||
});
|
||||
|
||||
it("clears resolved proof diagnostics when adding proof", async () => {
|
||||
const store = new WorkboardStore(createMemoryStore());
|
||||
const card = await store.create({
|
||||
|
||||
@@ -310,6 +310,13 @@ describe("workboard tools", () => {
|
||||
).rejects.toThrow(/claimed by main/);
|
||||
expect(await store.list()).toHaveLength(1);
|
||||
|
||||
await expect(
|
||||
otherTools.get("workboard_create")?.execute("call-2b", {
|
||||
title: "Wrong token child",
|
||||
parents: [parent.id],
|
||||
token: "test-token-placeholder",
|
||||
}),
|
||||
).rejects.toThrow(/claimed by main/);
|
||||
await otherTools.get("workboard_create")?.execute("call-2", {
|
||||
title: "Scoped child",
|
||||
parents: [parent.id],
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import { jsonResult, readStringParam } from "openclaw/plugin-sdk/core";
|
||||
import type { AnyAgentTool, OpenClawPluginApi } from "openclaw/plugin-sdk/plugin-entry";
|
||||
import type { OpenClawPluginToolContext } from "openclaw/plugin-sdk/plugin-entry";
|
||||
import { safeEqualSecret } from "openclaw/plugin-sdk/security-runtime";
|
||||
import { Type } from "typebox";
|
||||
import { WorkboardStore } from "./store.js";
|
||||
import type { WorkboardCard } from "./types.js";
|
||||
@@ -18,10 +19,7 @@ function contextOwner(ctx: OpenClawPluginToolContext | undefined): string {
|
||||
|
||||
function canMutateCard(card: WorkboardCard, ownerId: string, token?: string): boolean {
|
||||
const claim = card.metadata?.claim;
|
||||
if (!claim) {
|
||||
return true;
|
||||
}
|
||||
return claim.ownerId === ownerId || (Boolean(token) && claim.token === token);
|
||||
return !claim || claim.ownerId === ownerId || safeEqualSecret(token, claim.token);
|
||||
}
|
||||
|
||||
function readParentIds(value: unknown): string[] {
|
||||
|
||||
Reference in New Issue
Block a user