Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- Scope the Optio action audit trail to a workspace.
--
-- The unified activity feed (GET /api/activity) previously returned every
-- tenant's actions to any authenticated user, and the logged `params` jsonb
-- could carry other tenants' resource ids (and, before the write-time
-- allowlist, request-body values). Actions are now stamped with the caller's
-- workspace so the feed can filter by tenant.
--
-- Existing rows keep workspace_id = NULL. NULL is treated as an
-- operator/legacy action with no tenant context and is only surfaced to
-- admins by the activity feed (deny-by-default) — never to members/viewers.

ALTER TABLE "optio_actions" ADD COLUMN "workspace_id" uuid;--> statement-breakpoint
CREATE INDEX "optio_actions_workspace_id_idx" ON "optio_actions" ("workspace_id");
7 changes: 7 additions & 0 deletions apps/api/src/db/migrations/meta/_journal.json
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,13 @@
"when": 1784588891000,
"tag": "1784588891_backfill_task_workspace_id",
"breakpoints": true
},
{
"idx": 79,
"version": "7",
"when": 1785714869000,
"tag": "1785714869_optio_actions_workspace_id",
"breakpoints": true
}
]
}
6 changes: 5 additions & 1 deletion apps/api/src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -945,9 +945,12 @@ export const optioActions = pgTable(
"optio_actions",
{
id: uuid("id").primaryKey().defaultRandom(),
// Nullable: NULL rows are operator/legacy actions with no tenant context and
// are only surfaced to admins by the activity feed (deny-by-default).
workspaceId: uuid("workspace_id"),
userId: uuid("user_id").references(() => users.id),
action: text("action").notNull(), // tool name e.g. "retry_task", "bulk_cancel_active"
params: jsonb("params").$type<Record<string, unknown>>(), // sanitized tool call parameters
params: jsonb("params").$type<Record<string, unknown>>(), // allowlisted, non-secret tool call parameters
result: jsonb("result").$type<Record<string, unknown>>(), // outcome: affected IDs, error, etc.
success: boolean("success").notNull(),
conversationSnippet: text("conversation_snippet"), // user message that triggered this
Expand All @@ -957,6 +960,7 @@ export const optioActions = pgTable(
index("optio_actions_user_id_idx").on(table.userId),
index("optio_actions_action_idx").on(table.action),
index("optio_actions_created_at_idx").on(table.createdAt.desc()),
index("optio_actions_workspace_id_idx").on(table.workspaceId),
],
);

Expand Down
128 changes: 128 additions & 0 deletions apps/api/src/routes/activity.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import type { FastifyInstance } from "fastify";
import type { SQL } from "drizzle-orm";
import { PgDialect } from "drizzle-orm/pg-core";
import { buildRouteTestApp } from "../test-utils/build-route-test-app.js";

const mockDbExecute = vi.fn();
Expand Down Expand Up @@ -152,3 +154,129 @@ describe("GET /api/activity", () => {
expect(res.json().error).toBe("Failed to fetch activity feed");
});
});

describe("GET /api/activity — workspace isolation & param whitelisting", () => {
const dialect = new PgDialect();

/** Compile the SQL fragment handed to the mocked db.execute into text + params. */
function compile(call: unknown): { text: string; params: unknown[] } {
const q = dialect.sqlToQuery(call as SQL);
return { text: q.sql, params: q.params };
}

beforeEach(() => {
vi.clearAllMocks();
// requireRole("member") must actually enforce — never let a stray env
// disable auth for these assertions.
delete process.env.OPTIO_AUTH_DISABLED;
});

it("scopes a member strictly to their own workspace (no null-workspace rows)", async () => {
mockDbExecute
.mockResolvedValueOnce([])
.mockResolvedValueOnce([{ total: 0 }])
.mockResolvedValueOnce([]);

const app = await buildRouteTestApp((await import("./activity.js")).activityRoutes, {
user: { id: "u-A", workspaceId: "ws-A", workspaceRole: "member" },
});
const res = await app.inject({ method: "GET", url: "/api/activity?type=action" });
expect(res.statusCode).toBe(200);

const { text, params } = compile(mockDbExecute.mock.calls[0][0]);
// Query is scoped by workspace_id and bound to the caller's workspace only.
expect(text).toContain("workspace_id");
expect(params).toContain("ws-A");
// Members get strict equality — NOT the admin `OR workspace_id IS NULL` branch.
expect(text.toLowerCase()).not.toContain("is null");
});

it("lets an admin additionally see legacy null-workspace rows", async () => {
mockDbExecute
.mockResolvedValueOnce([])
.mockResolvedValueOnce([{ total: 0 }])
.mockResolvedValueOnce([]);

const app = await buildRouteTestApp((await import("./activity.js")).activityRoutes, {
user: { id: "u-A", workspaceId: "ws-A", workspaceRole: "admin" },
});
const res = await app.inject({ method: "GET", url: "/api/activity?type=action" });
expect(res.statusCode).toBe(200);

const { text, params } = compile(mockDbExecute.mock.calls[0][0]);
expect(params).toContain("ws-A");
// Admins see own-workspace + operator/legacy (null) rows.
expect(text.toLowerCase()).toContain("workspace_id is null");
});

it("does not leak another workspace's data: bound workspace is the caller's", async () => {
mockDbExecute
.mockResolvedValueOnce([])
.mockResolvedValueOnce([{ total: 0 }])
.mockResolvedValueOnce([]);

const app = await buildRouteTestApp((await import("./activity.js")).activityRoutes, {
user: { id: "u-A", workspaceId: "ws-A", workspaceRole: "member" },
});
await app.inject({ method: "GET", url: "/api/activity?type=action" });

const { params } = compile(mockDbExecute.mock.calls[0][0]);
// The only workspace ever bound is the caller's — workspace B is unreachable.
expect(params).toContain("ws-A");
expect(params).not.toContain("ws-B");
});

it("strips secret-bearing params from the response, keeping only allowlisted keys", async () => {
const now = new Date().toISOString();
mockDbExecute
.mockResolvedValueOnce([
{
id: "a1",
type: "action",
timestamp: now,
user_id: "u-A",
user_display_name: "Alice",
user_avatar_url: null,
action: "connection.update",
resource_type: "connection",
resource_id: "c1",
summary: "connection.update succeeded",
// Legacy row written before write-time filtering: full jsonb with secrets.
details: {
connectionId: "c1",
name: "prod-db",
apiToken: "SHOULD_NOT_LEAK",
password: "hunter2",
config: { url: "postgres://user:hunter2@db/app" },
crossTenantResourceId: "ws-B-secret-id",
},
},
])
.mockResolvedValueOnce([{ total: 1 }])
.mockResolvedValueOnce([{ type: "action", cnt: 1 }]);

const app = await buildRouteTestApp((await import("./activity.js")).activityRoutes, {
user: { id: "u-A", workspaceId: "ws-A", workspaceRole: "admin" },
});
const res = await app.inject({ method: "GET", url: "/api/activity" });
expect(res.statusCode).toBe(200);

const details = res.json().items[0].details;
expect(details).toEqual({ connectionId: "c1", name: "prod-db" });
// The whole serialized response must not carry the secret values.
const raw = res.payload;
expect(raw).not.toContain("SHOULD_NOT_LEAK");
expect(raw).not.toContain("hunter2");
expect(raw).not.toContain("crossTenantResourceId");
});

it("forbids viewers (read-only) from reading the audit feed", async () => {
const app = await buildRouteTestApp((await import("./activity.js")).activityRoutes, {
user: { id: "u-v", workspaceId: "ws-A", workspaceRole: "viewer" },
});
const res = await app.inject({ method: "GET", url: "/api/activity" });
expect(res.statusCode).toBe(403);
// Never touched the database.
expect(mockDbExecute).not.toHaveBeenCalled();
});
});
Loading
Loading