From 4426a9b76c697afa8fd98b5c7ebef73c1258da85 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Wed, 10 Jun 2026 22:58:03 +0200 Subject: [PATCH 1/2] fix(api): scope dashboard audit-log reads to the current account Dashboard summary and activity queries filtered auditLog by userId only, so a multi-account user saw events from every account they belong to. Queries now also require targetAccountId = current account (or null for user-level events), threaded from the auth context. Guardrail: new lint-meta source-text rule audit-log-read-account-scoped flags any src/ auditLog read filtered by userId without targetAccountId scoping; covered in tests/lint-meta and the rule docs/catalog are regenerated. Audit: F001 --- apps/api/scripts/lint-meta/RULES.md | 1 + apps/api/scripts/lint-meta/cli.ts | 2 + apps/api/scripts/lint-meta/registry.ts | 2 + .../audit-log-read-account-scoped.ts | 70 +++++++++++++ .../api/src/api/dashboard/dashboard.routes.ts | 24 +++-- .../src/api/dashboard/dashboard.service.ts | 42 ++++++-- .../api/dashboard/dashboard.service.test.ts | 98 +++++++++++++++++-- apps/api/tests/lint-meta/lint-meta.test.ts | 66 +++++++++++++ apps/docs/src/data/lint-meta-catalog.json | 6 ++ 9 files changed, 287 insertions(+), 24 deletions(-) create mode 100644 apps/api/scripts/lint-meta/rules/source-text/audit-log-read-account-scoped.ts diff --git a/apps/api/scripts/lint-meta/RULES.md b/apps/api/scripts/lint-meta/RULES.md index 35219faf..fba5b3da 100644 --- a/apps/api/scripts/lint-meta/RULES.md +++ b/apps/api/scripts/lint-meta/RULES.md @@ -44,6 +44,7 @@ Run `bun run lint:meta --list-rules` for the machine-readable list from the regi | `docs-no-retired-credentials` | source-text | no | Documentation prose must not reference retired default credentials. | | `external-client-timeout` | source-text | no | SDK clients (Stripe/OpenAI/Anthropic) need a timeout option; email transports (Resend/SendGrid/nodemailer) must be bounded; fetch() in src needs an AbortSignal. | | `no-raw-role-literal` | source-text | no | Use ROLE.* from acl.constants.ts instead of raw owner/admin/member/viewer string literals. | +| `audit-log-read-account-scoped` | source-text | no | Queries filtering auditLog by userId must also reference auditLog.targetAccountId — userId-only reads bleed a multi-account user's events across tenant boundaries. | | `routes-require-test-sibling` | testing | no | Route modules must ship with a matching HTTP-level test under tests/api/. | | `logic-files-require-test-sibling` | testing | no | Logic modules must ship with a matching tests/**/*.test.ts sibling. | | `lint-meta-rules-self-covered` | testing | no | Every lint-meta rule module must re-export its check function from cli.ts and carry a describe() test block — the guardrails must themselves be guarded. | diff --git a/apps/api/scripts/lint-meta/cli.ts b/apps/api/scripts/lint-meta/cli.ts index 1ff0e402..c3762427 100644 --- a/apps/api/scripts/lint-meta/cli.ts +++ b/apps/api/scripts/lint-meta/cli.ts @@ -48,6 +48,7 @@ import { checkSecurityScannerVersionParity, } from "./rules/ci/security-scanner-version-parity"; import { checkTofuBootstrapHardening } from "./rules/ci/tofu-bootstrap-hardening"; +import { checkAuditLogReadAccountScoped } from "./rules/source-text/audit-log-read-account-scoped"; import { checkCanonicalHelpersSingleHome } from "./rules/source-text/canonical-helpers-single-home"; import { checkDocsNoRetiredCredentials } from "./rules/source-text/docs-no-retired-credentials"; import { checkExternalClientTimeouts } from "./rules/source-text/external-client-timeout"; @@ -113,6 +114,7 @@ export { collectSourceFiles, findWorkflows } from "./context"; export { parseDotenvKeys } from "./parsers/dotenv"; export { parseTypeboxEnvSchemaKeys as parseEnvSchemaKeys } from "./parsers/typebox-env-schema"; export { + checkAuditLogReadAccountScoped, checkCanonicalHelpersSingleHome, checkDependencyPairs, checkDocsNoRetiredCredentials, diff --git a/apps/api/scripts/lint-meta/registry.ts b/apps/api/scripts/lint-meta/registry.ts index 0818143e..62b29583 100644 --- a/apps/api/scripts/lint-meta/registry.ts +++ b/apps/api/scripts/lint-meta/registry.ts @@ -21,6 +21,7 @@ import { eslintOverridePathsExistRule } from "./rules/config/eslint-override-pat import { tsconfigIncludePathsExistRule } from "./rules/config/tsconfig-include-paths-exist"; import { envCascadeDriftRule } from "./rules/env/env-cascade-drift"; import { noDirectProcessEnvRule } from "./rules/env/no-direct-process-env"; +import { auditLogReadAccountScopedRule } from "./rules/source-text/audit-log-read-account-scoped"; import { canonicalHelpersSingleHomeRule } from "./rules/source-text/canonical-helpers-single-home"; import { docsNoRetiredCredentialsRule } from "./rules/source-text/docs-no-retired-credentials"; import { externalClientTimeoutRule } from "./rules/source-text/external-client-timeout"; @@ -65,6 +66,7 @@ export const META_RULES: readonly IMetaRule[] = [ docsNoRetiredCredentialsRule, externalClientTimeoutRule, noRawRoleLiteralsRule, + auditLogReadAccountScopedRule, routesRequireTestSiblingRule, logicFilesRequireTestSiblingRule, lintMetaRulesSelfCoveredRule, diff --git a/apps/api/scripts/lint-meta/rules/source-text/audit-log-read-account-scoped.ts b/apps/api/scripts/lint-meta/rules/source-text/audit-log-read-account-scoped.ts new file mode 100644 index 00000000..aa1696fb --- /dev/null +++ b/apps/api/scripts/lint-meta/rules/source-text/audit-log-read-account-scoped.ts @@ -0,0 +1,70 @@ +import { readFileSync } from "node:fs"; + +import type { IMetaRule, IViolation } from "../../types"; + +/* + * Audit-log reads are tenant data. A query that filters auditLog by + * userId alone returns the user's events from EVERY account they belong + * to, bleeding account-level activity (billing changes, invitations, + * feature overrides) across tenant boundaries — the dashboard feed + * shipped exactly this defect. The schema carries an indexed + * targetAccountId for scoping, so any file that filters on + * eq(auditLog.userId, …) must also reference auditLog.targetAccountId + * in its query. Write paths (inserts) and join conditions like + * eq(users.id, auditLog.userId) don't match the filter pattern, so the + * check stays precise without an allowlist. + */ +const RULE = "audit-log-read-account-scoped"; +const USER_FILTER_RE = /eq\(auditLog\.userId\b/u; +const ACCOUNT_SCOPE_TOKEN = "auditLog.targetAccountId"; + +export function checkAuditLogReadAccountScoped( + root: string, + sourceFiles: readonly string[] +): IViolation[] { + const violations: IViolation[] = []; + + for (const file of sourceFiles) { + const relative = file.startsWith(root) ? file.slice(root.length + 1) : file; + + if (!relative.startsWith("src/") || relative.endsWith(".test.ts")) { + continue; + } + + const content = readFileSync(file, "utf8"); + + if ( + !USER_FILTER_RE.test(content) || + content.includes(ACCOUNT_SCOPE_TOKEN) + ) { + continue; + } + + const lines = content.split("\n"); + + for (let index = 0; index < lines.length; index++) { + if (!USER_FILTER_RE.test(lines[index] ?? "")) { + continue; + } + + violations.push({ + file, + rule: RULE, + message: `Line ${String(index + 1)}: auditLog query filters by userId without targetAccountId scoping — a multi-account user sees events from every account. Add an auditLog.targetAccountId clause (or isNull for user-level events).`, + }); + } + } + + return violations; +} + +/** auditLog reads filtered by userId must also scope on targetAccountId. */ +export const auditLogReadAccountScopedRule: IMetaRule = { + id: "audit-log-read-account-scoped", + category: "source-text", + description: + "Queries filtering auditLog by userId must also reference auditLog.targetAccountId — userId-only reads bleed a multi-account user's events across tenant boundaries.", + run({ root, sourceFiles }) { + return checkAuditLogReadAccountScoped(root, sourceFiles); + }, +}; diff --git a/apps/api/src/api/dashboard/dashboard.routes.ts b/apps/api/src/api/dashboard/dashboard.routes.ts index a68eebec..d2ea4584 100644 --- a/apps/api/src/api/dashboard/dashboard.routes.ts +++ b/apps/api/src/api/dashboard/dashboard.routes.ts @@ -12,19 +12,25 @@ const dashboardRoutes = requireAuth() .onError(({ code, error, set }) => errorHandler({ code: String(code), error, set }) ) - .get("/summary", async ({ user }) => dashboardService.getSummary(user.id), { - response: DashboardSummarySchema, - detail: { - tags: ["Dashboard"], - summary: "Dashboard summary stats (per user)", - security: [{ cookieAuth: [] }], - }, - }) + .get( + "/summary", + async ({ user, accountId }) => + dashboardService.getSummary(user.id, accountId), + { + response: DashboardSummarySchema, + detail: { + tags: ["Dashboard"], + summary: "Dashboard summary stats (per user)", + security: [{ cookieAuth: [] }], + }, + } + ) .get( "/activity", - async ({ user, query }) => + async ({ user, accountId, query }) => dashboardService.getActivity( user.id, + accountId, parseDashboardLimit(query.limit), query.cursor ), diff --git a/apps/api/src/api/dashboard/dashboard.service.ts b/apps/api/src/api/dashboard/dashboard.service.ts index 9b439cdd..dddee185 100644 --- a/apps/api/src/api/dashboard/dashboard.service.ts +++ b/apps/api/src/api/dashboard/dashboard.service.ts @@ -1,4 +1,4 @@ -import { and, count, desc, eq, lt, or, type SQL } from "drizzle-orm"; +import { and, count, desc, eq, isNull, lt, or, type SQL } from "drizzle-orm"; import { db } from "../../clients/postgres"; import { auditLog } from "../../clients/postgres/schema"; @@ -7,11 +7,37 @@ import type { IActivityPage, IDashboardSummary } from "./dashboard.types"; import { formatActivityTitle } from "./dashboard.utils"; export class DashboardService { - async getSummary(userId: string): Promise { + /* + * Tenant scope for the feed: the user's own events in the current + * account, plus their user-level events (null targetAccountId — logins, + * profile changes). Events targeting OTHER accounts the user belongs to + * must never leak into this account's dashboard. + */ + private scopedUserFilters(userId: string, accountId: string): SQL[] { + const filters: SQL[] = [eq(auditLog.userId, userId)]; + + const accountScope = or( + eq(auditLog.targetAccountId, accountId), + isNull(auditLog.targetAccountId) + ); + + if (accountScope !== undefined) { + filters.push(accountScope); + } + + return filters; + } + + async getSummary( + userId: string, + accountId: string + ): Promise { + const scope = and(...this.scopedUserFilters(userId, accountId)); + const [eventCountRow] = await db .select({ value: count() }) .from(auditLog) - .where(eq(auditLog.userId, userId)); + .where(scope); const recent = await db .select({ @@ -21,7 +47,7 @@ export class DashboardService { createdAt: auditLog.createdAt, }) .from(auditLog) - .where(eq(auditLog.userId, userId)) + .where(scope) .orderBy(desc(auditLog.createdAt)) .limit(5); @@ -37,15 +63,19 @@ export class DashboardService { async getActivity( userId: string, + accountId: string, limit: number, cursor?: string ): Promise { - const filters: SQL[] = [eq(auditLog.userId, userId)]; + const filters: SQL[] = this.scopedUserFilters(userId, accountId); if (cursor !== undefined && cursor !== "") { const cursorId = cursor.replace(/^cursor:/, ""); const cursorRow = await db.query.auditLog.findFirst({ - where: and(eq(auditLog.id, cursorId), eq(auditLog.userId, userId)), + where: and( + eq(auditLog.id, cursorId), + ...this.scopedUserFilters(userId, accountId) + ), }); if (cursorRow === undefined) { diff --git a/apps/api/tests/api/dashboard/dashboard.service.test.ts b/apps/api/tests/api/dashboard/dashboard.service.test.ts index 34bc0baf..ff1b5db0 100644 --- a/apps/api/tests/api/dashboard/dashboard.service.test.ts +++ b/apps/api/tests/api/dashboard/dashboard.service.test.ts @@ -11,6 +11,13 @@ import { dashboardService } from "../../../src/api/dashboard/dashboard.service"; const LOGIN = "user.login"; const LOGOUT = "user.logout"; +/* + * targetAccountId carries no FK, so fixed UUIDs stand in for two + * accounts the user belongs to without seeding account rows. + */ +const ACCOUNT_A = "00000000-0000-4000-8000-00000000000a"; +const ACCOUNT_B = "00000000-0000-4000-8000-00000000000b"; + const insertTestUser = async (suffix: string): Promise => { const [created] = await db .insert(users) @@ -32,6 +39,7 @@ const insertAudit = async (input: { userId: string | null; action: string; resource?: string; + targetAccountId?: string; }): Promise => { const [row] = await db .insert(auditLog) @@ -39,6 +47,7 @@ const insertAudit = async (input: { userId: input.userId, action: input.action, resource: input.resource ?? null, + targetAccountId: input.targetAccountId ?? null, }) .returning({ id: auditLog.id }); @@ -81,8 +90,8 @@ describe("DashboardService.getSummary user isolation", () => { await insertAudit({ userId: other, action: LOGOUT }); await insertAudit({ userId: null, action: "system.cron" }); - const mine = await dashboardService.getSummary(me); - const theirs = await dashboardService.getSummary(other); + const mine = await dashboardService.getSummary(me, ACCOUNT_A); + const theirs = await dashboardService.getSummary(other, ACCOUNT_A); expect(mine.totalEvents).toBe(2); expect(theirs.totalEvents).toBe(3); @@ -101,11 +110,39 @@ describe("DashboardService.getSummary user isolation", () => { await insertAudit({ userId: other, action: LOGIN }); await insertAudit({ userId: other, action: LOGOUT }); - const summary = await dashboardService.getSummary(me); + const summary = await dashboardService.getSummary(me, ACCOUNT_A); expect(summary.recentActivity).toHaveLength(1); expect(summary.recentActivity[0]?.id).toBe(myRowId); }); + + test("excludes the user's own events from other accounts, keeps user-level events", async () => { + if (!(await requireDb())) { + return; + } + + const me = await insertTestUser("me-multi-account"); + + const inAccountA = await insertAudit({ + userId: me, + action: "billing.update", + targetAccountId: ACCOUNT_A, + }); + const userLevel = await insertAudit({ userId: me, action: LOGIN }); + + await insertAudit({ + userId: me, + action: "invitation.create", + targetAccountId: ACCOUNT_B, + }); + + const summary = await dashboardService.getSummary(me, ACCOUNT_A); + + expect(summary.totalEvents).toBe(2); + expect(summary.recentActivity.map((row) => row.id).sort()).toEqual( + [inAccountA, userLevel].sort() + ); + }); }); describe("DashboardService.getActivity user isolation", () => { @@ -144,7 +181,7 @@ describe("DashboardService.getActivity user isolation", () => { await insertAudit({ userId: other, action: "user.event.x" }); await insertAudit({ userId: other, action: "user.event.y" }); - const page = await dashboardService.getActivity(me, 10); + const page = await dashboardService.getActivity(me, ACCOUNT_A, 10); expect(page.items).toHaveLength(3); expect(page.items.every((item) => mineIds.includes(item.id))).toBe(true); @@ -168,7 +205,12 @@ describe("DashboardService.getActivity user isolation", () => { let caught: unknown; try { - await dashboardService.getActivity(me, 10, `cursor:${otherId}`); + await dashboardService.getActivity( + me, + ACCOUNT_A, + 10, + `cursor:${otherId}` + ); } catch (error) { caught = error; } @@ -180,6 +222,44 @@ describe("DashboardService.getActivity user isolation", () => { } }); + test("excludes other-account rows from the feed and rejects their cursors", async () => { + if (!(await requireDb())) { + return; + } + + const me = await insertTestUser("me-multi-list"); + + const visible = await insertAudit({ + userId: me, + action: "billing.update", + targetAccountId: ACCOUNT_A, + }); + const foreign = await insertAudit({ + userId: me, + action: "invitation.create", + targetAccountId: ACCOUNT_B, + }); + + const page = await dashboardService.getActivity(me, ACCOUNT_A, 10); + + expect(page.items.map((item) => item.id)).toEqual([visible]); + + let caught: unknown; + + try { + await dashboardService.getActivity( + me, + ACCOUNT_A, + 10, + `cursor:${foreign}` + ); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(Error); + }); + test("returns empty items and null nextCursor when no rows exist", async () => { if (!(await requireDb())) { return; @@ -187,7 +267,7 @@ describe("DashboardService.getActivity user isolation", () => { const me = await insertTestUser("empty-list"); - const page = await dashboardService.getActivity(me, 10); + const page = await dashboardService.getActivity(me, ACCOUNT_A, 10); expect(page.items).toEqual([]); expect(page.nextCursor).toBeNull(); @@ -204,7 +284,7 @@ describe("DashboardService.getActivity user isolation", () => { await insertAudit({ userId: me, action: `user.event.${index}` }); } - const page = await dashboardService.getActivity(me, 2); + const page = await dashboardService.getActivity(me, ACCOUNT_A, 2); expect(page.items).toHaveLength(2); expect(page.nextCursor).not.toBeNull(); @@ -223,7 +303,7 @@ describe("DashboardService.getActivity user isolation", () => { const second = await insertAudit({ userId: me, action: "second" }); - const summary = await dashboardService.getSummary(me); + const summary = await dashboardService.getSummary(me, ACCOUNT_A); expect(summary.recentActivity[0]?.id).toBe(second); expect(summary.recentActivity[1]?.id).toBe(first); @@ -254,7 +334,7 @@ describe("DashboardService.getSummary edge cases", () => { const me = await insertTestUser("fresh"); - const summary = await dashboardService.getSummary(me); + const summary = await dashboardService.getSummary(me, ACCOUNT_A); expect(summary.totalEvents).toBe(0); expect(summary.recentActivity).toEqual([]); diff --git a/apps/api/tests/lint-meta/lint-meta.test.ts b/apps/api/tests/lint-meta/lint-meta.test.ts index 59124a79..3a34d7cf 100644 --- a/apps/api/tests/lint-meta/lint-meta.test.ts +++ b/apps/api/tests/lint-meta/lint-meta.test.ts @@ -14,6 +14,7 @@ import { tmpdir } from "node:os"; import { renderRulesMd } from "../../scripts/lint-meta/generate-rules-md"; import { + checkAuditLogReadAccountScoped, checkCanonicalHelpersSingleHome, checkDependencyPairs, checkDockerfileBaseImageShaPin, @@ -1150,6 +1151,71 @@ describe("checkWorkflowExpressionSyntax", () => { const CONTRACT_MD = "AGENT_CONTRACT.md"; const PKG_JSON = "package.json"; +describe("checkAuditLogReadAccountScoped", () => { + test("flags userId-only auditLog reads; passes account-scoped and write-path files", () => { + const root = mkdtempSync(join(tmpdir(), "lint-meta-audit-scope-")); + + try { + mkdirSync(join(root, "src"), { recursive: true }); + + const bad = join(root, "src", "bad.service.ts"); + + writeFileSync( + bad, + "const rows = await db\n .select()\n .from(auditLog)\n .where(eq(auditLog.userId, userId));\n" + ); + + const violations = checkAuditLogReadAccountScoped(root, [bad]); + + expect(violations.map((row) => row.rule)).toEqual([ + "audit-log-read-account-scoped", + ]); + + const good = join(root, "src", "good.service.ts"); + + writeFileSync( + good, + "const rows = await db\n .select()\n .from(auditLog)\n .where(\n and(\n eq(auditLog.userId, userId),\n or(\n eq(auditLog.targetAccountId, accountId),\n isNull(auditLog.targetAccountId)\n )\n )\n );\n" + ); + + expect(checkAuditLogReadAccountScoped(root, [good])).toEqual([]); + + const writer = join(root, "src", "writer.service.ts"); + + writeFileSync( + writer, + "await db.insert(auditLog).values({ userId, action, resource });\n" + ); + + expect(checkAuditLogReadAccountScoped(root, [writer])).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test("ignores test files and files outside src/", () => { + const root = mkdtempSync(join(tmpdir(), "lint-meta-audit-scope-")); + + try { + mkdirSync(join(root, "src"), { recursive: true }); + mkdirSync(join(root, "scripts"), { recursive: true }); + + const testFile = join(root, "src", "feed.service.test.ts"); + const scriptFile = join(root, "scripts", "report.ts"); + const body = "where(eq(auditLog.userId, userId));\n"; + + writeFileSync(testFile, body); + writeFileSync(scriptFile, body); + + expect( + checkAuditLogReadAccountScoped(root, [testFile, scriptFile]) + ).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +}); + describe("checkExternalClientTimeouts", () => { test("flags SDK constructors without timeout and bare fetch; passes bounded ones", () => { const root = mkdtempSync(join(tmpdir(), "lint-meta-timeouts-")); diff --git a/apps/docs/src/data/lint-meta-catalog.json b/apps/docs/src/data/lint-meta-catalog.json index b1f003f5..e2ccee47 100644 --- a/apps/docs/src/data/lint-meta-catalog.json +++ b/apps/docs/src/data/lint-meta-catalog.json @@ -434,6 +434,12 @@ "ciCritical": false, "description": "Use ROLE.* from acl.constants.ts instead of raw owner/admin/member/viewer string literals." }, + { + "id": "audit-log-read-account-scoped", + "category": "source-text", + "ciCritical": false, + "description": "Queries filtering auditLog by userId must also reference auditLog.targetAccountId — userId-only reads bleed a multi-account user's events across tenant boundaries." + }, { "id": "routes-require-test-sibling", "category": "testing", From b74610ca458d7f5f97b8c42d7c651c4def2006b1 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Wed, 10 Jun 2026 22:58:53 +0200 Subject: [PATCH 2/2] perf(ui): memoize AppSidebar nav items useAppSidebar rebuilt the icon record and items array on every render, giving the items prop a new identity each time and defeating memoization in the sidebar subtree. The computation is now inside useMemo keyed on [showBilling, t]. Audit: F002 --- .../core/AppSidebar/AppSidebar.hooks.ts | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/apps/ui/src/components/core/AppSidebar/AppSidebar.hooks.ts b/apps/ui/src/components/core/AppSidebar/AppSidebar.hooks.ts index 16b4897a..7f022d7d 100644 --- a/apps/ui/src/components/core/AppSidebar/AppSidebar.hooks.ts +++ b/apps/ui/src/components/core/AppSidebar/AppSidebar.hooks.ts @@ -1,3 +1,5 @@ +import { useMemo } from "react"; + import { Bell, CreditCard, @@ -32,24 +34,26 @@ export function useAppSidebar(props: IAppSidebarProps): IAppSidebarView { capabilities.data?.features.billing.enabled === true && me.data?.role === ROLE.owner; - const icons: Record = { - dashboard: LayoutDashboard, - notifications: Bell, - team: Users, - auditLog: History, - settings: Settings, - billing: CreditCard, - profile: User - }; + const items: IAppSidebarNavItemView[] = useMemo(() => { + const icons: Record = { + dashboard: LayoutDashboard, + notifications: Bell, + team: Users, + auditLog: History, + settings: Settings, + billing: CreditCard, + profile: User + }; - const items: IAppSidebarNavItemView[] = APP_SIDEBAR_NAV_ITEMS.filter( - (item) => item.id !== "billing" || showBilling - ).map((item) => ({ - id: item.id, - path: item.path, - label: t(item.labelKey), - icon: icons[item.id] - })); + return APP_SIDEBAR_NAV_ITEMS.filter( + (item) => item.id !== "billing" || showBilling + ).map((item) => ({ + id: item.id, + path: item.path, + label: t(item.labelKey), + icon: icons[item.id] + })); + }, [showBilling, t]); return { className: props.className,