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
4 changes: 2 additions & 2 deletions templates/clips/actions/add-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export default defineAction({
.optional()
.describe("Organization members mentioned in the comment"),
}),
run: async (args) => {
await assertCommentAccess(args.recordingId);
run: async (args, ctx) => {
await assertCommentAccess(args.recordingId, "write", ctx?.requestHeaders);

const authorEmail = getRequestUserEmail();
if (!authorEmail) {
Expand Down
199 changes: 180 additions & 19 deletions templates/clips/actions/comment-writing-access.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import { createHash } from "node:crypto";

import type { ActionRunContext } from "@agent-native/core/action";
import { signScopedAgentAccessToken } from "@agent-native/core/server";
import { signShortLivedToken } from "@agent-native/core/server/short-lived-token";
import { ForbiddenError } from "@agent-native/core/sharing";
import { beforeEach, describe, expect, it, vi } from "vitest";

Expand Down Expand Up @@ -96,43 +101,88 @@ import replyToComment from "./reply-to-comment";
import resolveComment from "./resolve-comment";
import updateComment from "./update-comment";

const protectedRecording = {
enableComments: true,
password: "fake-stored-password",
sharePasswordVersion: "initial",
};
function proofContext(kind?: string): ActionRunContext {
const scope = createHash("sha256")
.update(
JSON.stringify(["recording-1", protectedRecording.password, "initial"]),
)
.digest("hex");
let resourceId = `clips-comment:recording-1:${scope}`;
if (kind === "media") resourceId = "recording-1";

if (kind === "wrong-record")
resourceId = `clips-comment:recording-2:${scope}`;
if (kind === "rotated") resourceId = "clips-comment:recording-1:old-version";
let token = signShortLivedToken({
resourceId,
ttlSeconds: kind === "expired" ? -1 : 600,
});
if (kind === "agent")
token = signScopedAgentAccessToken({
resourceKind: "clip-agent-context",
resourceId: "recording-1",
});
if (kind === "malformed") token = "malformed";
if (kind === "tampered") token = token.slice(0, -5) + "AAAAA";
return {
requestHeaders: new Headers(kind ? { "x-clips-comment-token": token } : {}),
};
}

const writers = [
{
name: "new comment",
run: () =>
addComment.run({
recordingId: "recording-1",
content: "New text",
videoTimestampMs: 0,
}),
run: (ctx?: ActionRunContext) =>
addComment.run(
{
recordingId: "recording-1",
content: "New text",
videoTimestampMs: 0,
},
ctx,
),
},
{
name: "reply through add-comment",
run: () =>
addComment.run({
recordingId: "recording-1",
content: "Reply text",
videoTimestampMs: 12542,
parentId: "comment-1",
threadId: "thread-1",
}),
run: (ctx?: ActionRunContext) =>
addComment.run(
{
recordingId: "recording-1",
content: "Reply text",
videoTimestampMs: 12542,
parentId: "comment-1",
threadId: "thread-1",
},
ctx,
),
},
{
name: "reply-to-comment",
run: () =>
replyToComment.run({ commentId: "comment-1", content: "Reply text" }),
run: (ctx?: ActionRunContext) =>
replyToComment.run(
{ commentId: "comment-1", content: "Reply text" },
ctx,
),
},
{
name: "edit",
run: () => updateComment.run({ id: "comment-1", content: "Updated text" }),
run: (ctx?: ActionRunContext) =>
updateComment.run({ id: "comment-1", content: "Updated text" }, ctx),
},
{
name: "reaction",
run: () => reactToComment.run({ commentId: "comment-1", emoji: "👍" }),
run: (ctx?: ActionRunContext) =>
reactToComment.run({ commentId: "comment-1", emoji: "👍" }, ctx),
},
{
name: "resolve",
run: () => resolveComment.run({ id: "comment-1", resolved: true }),
run: (ctx?: ActionRunContext) =>
resolveComment.run({ id: "comment-1", resolved: true }, ctx),
},
];

Expand All @@ -151,6 +201,82 @@ beforeEach(() => {
});

describe.each(writers)("$name comment writing access", ({ run }) => {
it.each([
undefined,
"malformed",
"media",
"agent",
"wrong-record",
"expired",
"rotated",
"tampered",
])(
"refuses password-protected writes with %s proof before side effects",
async (kind) => {
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: protectedRecording,
});
await expect(run(proofContext(kind))).rejects.toBeInstanceOf(
ForbiddenError,
);
expect(state.inserted).toEqual([]);
expect(mockUpdate).not.toHaveBeenCalled();
expect(mockNotify).not.toHaveBeenCalled();
expect(mockWriteAppState).not.toHaveBeenCalled();
},
);

it("accepts a purpose-scoped password grant", async () => {
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: protectedRecording,
});
await expect(run(proofContext("valid"))).resolves.toBeDefined();
expect(state.inserted.length + mockUpdate.mock.calls.length).toBe(1);
});

it.each([
{ password: "fake-new-stored-password" },
{ sharePasswordVersion: "fake-next-version" },
])("invalidates proof after password changes: %j", async (change) => {
const ctx = proofContext("valid");
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: { ...protectedRecording, ...change },
});
await expect(run(ctx)).rejects.toBeInstanceOf(ForbiddenError);
expect(state.inserted).toEqual([]);
expect(mockUpdate).not.toHaveBeenCalled();
});

it("lets the owner write without password proof", async () => {
mockAssertAccess.mockResolvedValue({
role: "owner",
resource: protectedRecording,
});
await expect(run()).resolves.toBeDefined();
});

it.each([
{ enableComments: false },
{ trashedAt: "2026-01-01T00:00:00Z" },
{ expiresAt: "2000-01-01T00:00:00Z" },
])(
"keeps lifecycle restrictions with a valid grant: %j",
async (overrides) => {
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: { ...protectedRecording, ...overrides },
});
await expect(run(proofContext("valid"))).rejects.toBeInstanceOf(
ForbiddenError,
);
expect(state.inserted).toEqual([]);
expect(mockUpdate).not.toHaveBeenCalled();
},
);

it.each(["viewer", "owner"])(
"refuses disabled comments for a %s before side effects",
async (role) => {
Expand Down Expand Up @@ -231,3 +357,38 @@ it("replies retain the parent thread, organization, frame, and return contract",
}),
]);
});

it.each([undefined, null, "legacy:fake"])(
"binds legacy comment proof to the stored password at version %s",
async (sharePasswordVersion) => {
const resource = { ...protectedRecording, sharePasswordVersion };
mockAssertAccess.mockResolvedValue({ role: "viewer", resource });
const scope = createHash("sha256")
.update(
JSON.stringify([
"recording-1",
resource.password,
sharePasswordVersion ?? null,
]),
)
.digest("hex");
const token = signShortLivedToken({
resourceId: `clips-comment:recording-1:${scope}`,
ttlSeconds: 600,
});
const ctx = {
requestHeaders: new Headers({ "x-clips-comment-token": token }),
};
const args = {
recordingId: "recording-1",
content: "Fake comment",
videoTimestampMs: 0,
};
await expect(addComment.run(args, ctx)).resolves.toBeDefined();
resource.password = "fake-rotated-password";
await expect(addComment.run(args, ctx)).rejects.toBeInstanceOf(
ForbiddenError,
);
expect(state.inserted).toHaveLength(1);
},
);
47 changes: 47 additions & 0 deletions templates/clips/actions/delete-comment.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { createHash } from "node:crypto";

import { signShortLivedToken } from "@agent-native/core/server/short-lived-token";
import { ForbiddenError } from "@agent-native/core/sharing";
import { beforeEach, describe, expect, it, vi } from "vitest";

Expand Down Expand Up @@ -141,6 +144,23 @@ beforeEach(() => {
});

describe("delete-comment", () => {
it("refuses a password-protected author's deletion without proof", async () => {
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: {
enableComments: true,
password: "fake-password",
sharePasswordVersion: "initial",
},
});
const original = [...state.rows];
await expect(action.run({ id: "comment-1" })).rejects.toBeInstanceOf(
ForbiddenError,
);
expect(state.rows).toEqual(original);
expect(mockWriteAppState).not.toHaveBeenCalled();
});

it.each([
{ role: "viewer", email: "author@example.com" },
{ role: "editor", email: "editor@example.com" },
Expand Down Expand Up @@ -252,3 +272,30 @@ describe("delete-comment", () => {
]);
});
});

it("deletes the unlocked author's comment with purpose-scoped proof", async () => {
const password = "fake-stored-password";
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: {
enableComments: true,
password,
sharePasswordVersion: "initial",
},
});
const scope = createHash("sha256")
.update(JSON.stringify(["recording-1", password, "initial"]))
.digest("hex");
const token = signShortLivedToken({
resourceId: `clips-comment:recording-1:${scope}`,
ttlSeconds: 600,
});
await expect(
action.run(
{ id: "comment-1" },
{ requestHeaders: new Headers({ "x-clips-comment-token": token }) },
),
).resolves.toMatchObject({
deletedCommentIds: ["comment-1", "comment-1-reply", "comment-1-reply-2"],
});
});
8 changes: 6 additions & 2 deletions templates/clips/actions/delete-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default defineAction({
schema: z.object({
id: z.string().describe("Comment ID"),
}),
run: async (args) => {
run: async (args, ctx) => {
const db = getDb();
const [existing] = await db
.select()
Expand All @@ -27,7 +27,11 @@ export default defineAction({
const userEmail = getRequestUserEmail();
const isAuthor = sameOwnerEmail(existing.authorEmail, userEmail);

await assertCommentAccess(existing.recordingId);
await assertCommentAccess(
existing.recordingId,
"write",
ctx?.requestHeaders,
);

if (!isAuthor) {
try {
Expand Down
28 changes: 28 additions & 0 deletions templates/clips/actions/list-comments.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { createHash } from "node:crypto";

import { signShortLivedToken } from "@agent-native/core/server/short-lived-token";
import { ForbiddenError } from "@agent-native/core/sharing";
import { beforeEach, describe, expect, it, vi } from "vitest";

Expand Down Expand Up @@ -112,3 +115,28 @@ describe("list-comments recording policy", () => {
},
);
});

it("reads unlocked password comments with the same purpose-scoped proof", async () => {
const password = "fake-stored-password";
mockAssertAccess.mockResolvedValue({
role: "viewer",
resource: {
enableComments: true,
password,
sharePasswordVersion: "initial",
},
});
const scope = createHash("sha256")
.update(JSON.stringify(["recording-1", password, "initial"]))
.digest("hex");
const token = signShortLivedToken({
resourceId: `clips-comment:recording-1:${scope}`,
ttlSeconds: 600,
});
await expect(
listComments.run(
{ recordingId: "recording-1" },
{ requestHeaders: new Headers({ "x-clips-comment-token": token }) },
),
).resolves.toMatchObject({ comments: [{ id: "comment-1" }] });
});
4 changes: 2 additions & 2 deletions templates/clips/actions/list-comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ export default defineAction({
recordingId: z.string().describe("Recording ID"),
}),
http: { method: "GET" },
run: async (args) => {
await assertCommentAccess(args.recordingId, "read");
run: async (args, ctx) => {
await assertCommentAccess(args.recordingId, "read", ctx?.requestHeaders);

const db = getDb();
const rows = await db
Expand Down
Loading
Loading