From bde35c3518361519de5ab82bf46e2a1564054db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 15 May 2026 00:15:37 -0400 Subject: [PATCH] fix(security): scope social account references --- src/main/api-server/routes-misc.ts | 59 +++++-- src/main/database/entity-crud.ts | 7 +- .../migrations/v59-social-company-scope.ts | 163 ++++++++++++++++++ src/main/database/schema.ts | 22 ++- src/main/database/social.ts | 90 +++++++--- src/main/ipc-browser-handlers.ts | 11 +- tests/unit/database.test.ts | 2 + tests/unit/legacy-baseline-migration.test.ts | 4 + .../requires-filesystem-migration.test.ts | 3 +- 9 files changed, 317 insertions(+), 44 deletions(-) create mode 100644 src/main/database/migrations/v59-social-company-scope.ts diff --git a/src/main/api-server/routes-misc.ts b/src/main/api-server/routes-misc.ts index df44c611..549d5ba6 100644 --- a/src/main/api-server/routes-misc.ts +++ b/src/main/api-server/routes-misc.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { requireParam, type AgentApiServer } from "./server-core"; +import { SocialAccountScopeError } from "../database/social"; const socialDraftSchema = z.object({ socialAccountId: z.string().uuid(), @@ -9,6 +10,18 @@ const socialDraftSchema = z.object({ scheduledAt: z.string().datetime().optional(), }); +function writeSocialScopeError( + server: AgentApiServer, + res: Parameters[0], + error: unknown, +): boolean { + if (error instanceof SocialAccountScopeError) { + server.json(res, 422, { error: error.message, reference: error.reference }); + return true; + } + return false; +} + export function registerMiscRoutes(server: AgentApiServer) { const db = server.db; const onDomainChanged = server.onDomainChanged; @@ -51,15 +64,21 @@ export function registerMiscRoutes(server: AgentApiServer) { impactSummary: `Will ${input.actionType} on ${account.displayName || account.accountName}'s ${account.platform} account. Content will be publicly visible.`, }); - const actionId = db.core.social.saveBrowserAction({ - companyId, - socialAccountId: input.socialAccountId, - agentId: token.agentId, - actionType: input.actionType, - payloadJson: JSON.stringify(input.payload), - status: "approval_required", - approvalId, - }); + let actionId: string; + try { + actionId = db.core.social.saveBrowserAction({ + companyId, + socialAccountId: input.socialAccountId, + agentId: token.agentId, + actionType: input.actionType, + payloadJson: JSON.stringify(input.payload), + status: "approval_required", + approvalId, + }); + } catch (error) { + if (writeSocialScopeError(server, res, error)) return; + throw error; + } db.core.activity.addActivity({ companyId, @@ -75,14 +94,20 @@ export function registerMiscRoutes(server: AgentApiServer) { return; } - const actionId = db.core.social.saveBrowserAction({ - companyId, - socialAccountId: input.socialAccountId, - agentId: token.agentId, - actionType: input.actionType, - payloadJson: JSON.stringify(input.payload), - status: "queued", - }); + let actionId: string; + try { + actionId = db.core.social.saveBrowserAction({ + companyId, + socialAccountId: input.socialAccountId, + agentId: token.agentId, + actionType: input.actionType, + payloadJson: JSON.stringify(input.payload), + status: "queued", + }); + } catch (error) { + if (writeSocialScopeError(server, res, error)) return; + throw error; + } db.core.activity.addActivity({ companyId, diff --git a/src/main/database/entity-crud.ts b/src/main/database/entity-crud.ts index 9320a93b..880a0832 100644 --- a/src/main/database/entity-crud.ts +++ b/src/main/database/entity-crud.ts @@ -232,7 +232,12 @@ export function buildEntityCrudMethods( }, deleteSecret(id: string, companyId: string) { - getDb().prepare("delete from secrets where id = ? and company_id = ?").run(id, companyId); + transaction(() => { + getDb() + .prepare("update social_accounts set credential_secret_id = null, updated_at = ? where credential_secret_id = ? and company_id = ?") + .run(nowIso(), id, companyId); + getDb().prepare("delete from secrets where id = ? and company_id = ?").run(id, companyId); + }); }, countActiveGoalsForCompany(companyId: string): number { diff --git a/src/main/database/migrations/v59-social-company-scope.ts b/src/main/database/migrations/v59-social-company-scope.ts new file mode 100644 index 00000000..edcf7e2d --- /dev/null +++ b/src/main/database/migrations/v59-social-company-scope.ts @@ -0,0 +1,163 @@ +import type { DatabaseSync } from "node:sqlite"; +import { nowIso, tableExists } from "./types"; + +export const SOCIAL_COMPANY_SCOPE_SCHEMA_VERSION = 59; +export const SOCIAL_COMPANY_SCOPE_SCHEMA_NAME = "social_company_scope"; + +const TRIGGER_NAMES = [ + "trg_social_accounts_credential_secret_scope_insert", + "trg_social_accounts_credential_secret_scope_update", + "trg_browser_actions_social_account_scope_insert", + "trg_browser_actions_social_account_scope_update", +] as const; + +export function migrateSocialCompanyScopeV59(db: DatabaseSync): void { + cleanupInvalidSocialAccountSecretRefs(db); + cleanupInvalidBrowserActionRefs(db); + installSocialCompanyScopeTriggers(db); + db + .prepare("insert or replace into schema_migrations (version, name, applied_at) values (?, ?, ?)") + .run(SOCIAL_COMPANY_SCOPE_SCHEMA_VERSION, SOCIAL_COMPANY_SCOPE_SCHEMA_NAME, nowIso()); + assertSocialCompanyScopeV59(db); +} + +export function assertSocialCompanyScopeV59(db: DatabaseSync): void { + if (!tableExists(db, "social_accounts")) { + throw new Error("Schema v59 drift: social_accounts table is missing."); + } + if (!tableExists(db, "secrets")) { + throw new Error("Schema v59 drift: secrets table is missing."); + } + if (!tableExists(db, "browser_actions")) { + throw new Error("Schema v59 drift: browser_actions table is missing."); + } + const invalid = db + .prepare( + `select count(*) as count + from social_accounts sa + where sa.credential_secret_id is not null + and not exists ( + select 1 + from secrets s + where s.id = sa.credential_secret_id + and s.company_id = sa.company_id + )`, + ) + .get() as { count: number } | undefined; + if ((invalid?.count ?? 0) > 0) { + throw new Error("Schema v59 drift: social account credential secrets must belong to the same company."); + } + const invalidActions = db + .prepare( + `select count(*) as count + from browser_actions ba + where not exists ( + select 1 + from social_accounts sa + where sa.id = ba.social_account_id + and sa.company_id = ba.company_id + )`, + ) + .get() as { count: number } | undefined; + if ((invalidActions?.count ?? 0) > 0) { + throw new Error("Schema v59 drift: browser actions must reference social accounts in the same company."); + } + for (const triggerName of TRIGGER_NAMES) { + if (!triggerExists(db, triggerName)) { + throw new Error(`Schema v59 drift: ${triggerName} trigger is missing.`); + } + } +} + +function cleanupInvalidSocialAccountSecretRefs(db: DatabaseSync): void { + if (!tableExists(db, "social_accounts") || !tableExists(db, "secrets")) return; + db + .prepare( + `update social_accounts + set credential_secret_id = null, + updated_at = ? + where credential_secret_id is not null + and not exists ( + select 1 + from secrets s + where s.id = social_accounts.credential_secret_id + and s.company_id = social_accounts.company_id + )`, + ) + .run(nowIso()); +} + +function cleanupInvalidBrowserActionRefs(db: DatabaseSync): void { + if (!tableExists(db, "browser_actions") || !tableExists(db, "social_accounts")) return; + db + .prepare( + `delete from browser_actions + where not exists ( + select 1 + from social_accounts sa + where sa.id = browser_actions.social_account_id + and sa.company_id = browser_actions.company_id + )`, + ) + .run(); +} + +function installSocialCompanyScopeTriggers(db: DatabaseSync): void { + for (const triggerName of TRIGGER_NAMES) { + db.exec(`drop trigger if exists ${triggerName}`); + } + db.exec(` + create trigger trg_social_accounts_credential_secret_scope_insert + before insert on social_accounts + when new.credential_secret_id is not null + and not exists ( + select 1 from secrets + where id = new.credential_secret_id + and company_id = new.company_id + ) + begin + select raise(abort, 'social account credential secret must belong to the same company'); + end; + + create trigger trg_social_accounts_credential_secret_scope_update + before update of credential_secret_id, company_id on social_accounts + when new.credential_secret_id is not null + and not exists ( + select 1 from secrets + where id = new.credential_secret_id + and company_id = new.company_id + ) + begin + select raise(abort, 'social account credential secret must belong to the same company'); + end; + + create trigger trg_browser_actions_social_account_scope_insert + before insert on browser_actions + when not exists ( + select 1 from social_accounts + where id = new.social_account_id + and company_id = new.company_id + ) + begin + select raise(abort, 'browser action social account must belong to the same company'); + end; + + create trigger trg_browser_actions_social_account_scope_update + before update of social_account_id, company_id on browser_actions + when not exists ( + select 1 from social_accounts + where id = new.social_account_id + and company_id = new.company_id + ) + begin + select raise(abort, 'browser action social account must belong to the same company'); + end; + `); +} + +function triggerExists(db: DatabaseSync, name: string): boolean { + const row = db + .prepare("select name from sqlite_master where type = 'trigger' and name = ? limit 1") + .get(name) as { name: string } | undefined; + return Boolean(row); +} diff --git a/src/main/database/schema.ts b/src/main/database/schema.ts index b9a398e6..67117aa5 100644 --- a/src/main/database/schema.ts +++ b/src/main/database/schema.ts @@ -19,12 +19,17 @@ import { assertApprovalPoliciesV58, migrateApprovalPoliciesV58, } from "./migrations/v58-approval-policies"; +import { + SOCIAL_COMPANY_SCOPE_SCHEMA_VERSION, + assertSocialCompanyScopeV59, + migrateSocialCompanyScopeV59, +} from "./migrations/v59-social-company-scope"; import { type ReadAll, type Transaction, } from "./migrations/types"; -const LATEST_SCHEMA_VERSION = APPROVAL_POLICY_SCHEMA_VERSION; +const LATEST_SCHEMA_VERSION = SOCIAL_COMPANY_SCOPE_SCHEMA_VERSION; export function initializeDatabase( db: DatabaseSync, @@ -44,6 +49,17 @@ export function initializeDatabase( seedSettings(db); assertTaskLeasesV57(db); assertApprovalPoliciesV58(db); + assertSocialCompanyScopeV59(db); + }); + return; + } + + if (currentVersion === APPROVAL_POLICY_SCHEMA_VERSION) { + tx(() => { + seedSettings(db); + assertTaskLeasesV57(db); + assertApprovalPoliciesV58(db); + migrateSocialCompanyScopeV59(db); }); return; } @@ -53,6 +69,7 @@ export function initializeDatabase( seedSettings(db); assertTaskLeasesV57(db); migrateApprovalPoliciesV58(db); + migrateSocialCompanyScopeV59(db); }); return; } @@ -65,6 +82,7 @@ export function initializeDatabase( assertBaselineSchema(db); migrateTaskLeasesV57(db); migrateApprovalPoliciesV58(db); + migrateSocialCompanyScopeV59(db); }); return; } @@ -78,6 +96,7 @@ export function initializeDatabase( assertBaselineSchema(db); migrateTaskLeasesV57(db); migrateApprovalPoliciesV58(db); + migrateSocialCompanyScopeV59(db); }); return; } @@ -89,6 +108,7 @@ export function initializeDatabase( assertBaselineSchema(db); migrateTaskLeasesV57(db); migrateApprovalPoliciesV58(db); + migrateSocialCompanyScopeV59(db); }); } diff --git a/src/main/database/social.ts b/src/main/database/social.ts index 289579b3..90915274 100644 --- a/src/main/database/social.ts +++ b/src/main/database/social.ts @@ -2,7 +2,17 @@ import { randomUUID } from "node:crypto"; import type { DatabaseSync, SQLInputValue } from "node:sqlite"; import type { BrowserActionRecord, SocialAccountRecord } from "@shared/types"; import type { ReadAll, SqliteRow, Transaction } from "./helpers"; -import { ensureId, nowIso, toBoolean } from "./helpers"; +import { ensureId, normalizeOptionalId, nowIso, toBoolean } from "./helpers"; + +export class SocialAccountScopeError extends Error { + constructor( + public readonly reference: "social_account" | "credential_secret", + message: string, + ) { + super(message); + this.name = "SocialAccountScopeError"; + } +} export function buildSocialMethods( getDb: () => DatabaseSync, @@ -22,33 +32,61 @@ export function buildSocialMethods( requireApproval?: boolean; metadataJson?: string; }): string { - const id = ensureId(input.id); - const now = nowIso(); - getDb().prepare(`INSERT INTO social_accounts (id, company_id, platform, account_name, display_name, profile_url, credential_secret_id, status, require_approval, metadata_json, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(id) DO UPDATE SET - platform = excluded.platform, - account_name = excluded.account_name, - display_name = excluded.display_name, - profile_url = excluded.profile_url, - credential_secret_id = excluded.credential_secret_id, - status = excluded.status, - require_approval = excluded.require_approval, - metadata_json = excluded.metadata_json, - updated_at = excluded.updated_at`).run( - id, input.companyId, input.platform, input.accountName, - input.displayName ?? "", input.profileUrl ?? "", - input.credentialSecretId ?? null, input.status ?? "login_required", - input.requireApproval !== false ? 1 : 0, - input.metadataJson ?? "{}", now, now, - ); - return id; + return transaction(() => { + const id = ensureId(input.id); + const now = nowIso(); + const credentialSecretId = normalizeOptionalId(input.credentialSecretId); + const existing = getDb() + .prepare("SELECT id, company_id AS companyId FROM social_accounts WHERE id = ? LIMIT 1") + .get(id) as { id: string; companyId: string } | undefined; + if (existing && existing.companyId !== input.companyId) { + throw new SocialAccountScopeError("social_account", "Social account does not belong to the current company"); + } + if (credentialSecretId) { + const secret = getDb() + .prepare("SELECT id FROM secrets WHERE id = ? AND company_id = ? LIMIT 1") + .get(credentialSecretId, input.companyId) as { id: string } | undefined; + if (!secret) { + throw new SocialAccountScopeError("credential_secret", "Credential secret does not belong to the current company"); + } + } + if (existing) { + getDb().prepare(`UPDATE social_accounts + SET platform = ?, + account_name = ?, + display_name = ?, + profile_url = ?, + credential_secret_id = ?, + status = ?, + require_approval = ?, + metadata_json = ?, + updated_at = ? + WHERE id = ? AND company_id = ?`).run( + input.platform, input.accountName, + input.displayName ?? "", input.profileUrl ?? "", + credentialSecretId, input.status ?? "login_required", + input.requireApproval !== false ? 1 : 0, + input.metadataJson ?? "{}", now, + id, input.companyId, + ); + } else { + getDb().prepare(`INSERT INTO social_accounts (id, company_id, platform, account_name, display_name, profile_url, credential_secret_id, status, require_approval, metadata_json, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`).run( + id, input.companyId, input.platform, input.accountName, + input.displayName ?? "", input.profileUrl ?? "", + credentialSecretId, input.status ?? "login_required", + input.requireApproval !== false ? 1 : 0, + input.metadataJson ?? "{}", now, now, + ); + } + return id; + }); }, deleteSocialAccount(id: string, companyId: string) { transaction(() => { - getDb().prepare("DELETE FROM social_accounts WHERE id = ? AND company_id = ?").run(id, companyId); getDb().prepare("DELETE FROM browser_actions WHERE social_account_id = ? AND company_id = ?").run(id, companyId); + getDb().prepare("DELETE FROM social_accounts WHERE id = ? AND company_id = ?").run(id, companyId); }); }, @@ -91,6 +129,12 @@ export function buildSocialMethods( }): string { const id = randomUUID(); const now = nowIso(); + const socialAccount = getDb() + .prepare("SELECT id FROM social_accounts WHERE id = ? AND company_id = ? LIMIT 1") + .get(input.socialAccountId, input.companyId) as { id: string } | undefined; + if (!socialAccount) { + throw new SocialAccountScopeError("social_account", "Browser action social account does not belong to the current company"); + } getDb().prepare(`INSERT INTO browser_actions (id, company_id, social_account_id, agent_id, task_id, action_type, payload_json, status, approval_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`).run( id, input.companyId, input.socialAccountId, input.agentId, diff --git a/src/main/ipc-browser-handlers.ts b/src/main/ipc-browser-handlers.ts index 7d764b7a..cac802fe 100644 --- a/src/main/ipc-browser-handlers.ts +++ b/src/main/ipc-browser-handlers.ts @@ -7,6 +7,7 @@ import { triggerBrowserLoginSchema, } from "@shared/contracts"; import { fail, ok, type RegisterDomainHandlersDependencies } from "./ipc-domain-common"; +import { SocialAccountScopeError } from "./database/social"; const inflightBrowserLogins = new Set(); @@ -30,7 +31,15 @@ export function registerBrowserHandlers({ const input = socialAccountInputSchema.parse(payload); const err = ensureCompanyExists(input.companyId); if (err) return err; - const id = db.core.social.saveSocialAccount(input); + let id: string; + try { + id = db.core.social.saveSocialAccount(input); + } catch (error) { + if (error instanceof SocialAccountScopeError) { + return fail("INVALID_COMPANY_REFERENCE", error.message); + } + throw error; + } publishDomainChanged(); return ok(id); }); diff --git a/tests/unit/database.test.ts b/tests/unit/database.test.ts index e97e5b8a..c449b2ac 100644 --- a/tests/unit/database.test.ts +++ b/tests/unit/database.test.ts @@ -481,6 +481,7 @@ describe("AppDatabase", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); db.close(); @@ -494,6 +495,7 @@ describe("AppDatabase", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); }); diff --git a/tests/unit/legacy-baseline-migration.test.ts b/tests/unit/legacy-baseline-migration.test.ts index 953a4849..7461ce04 100644 --- a/tests/unit/legacy-baseline-migration.test.ts +++ b/tests/unit/legacy-baseline-migration.test.ts @@ -175,6 +175,7 @@ describe("legacy to v56 baseline migration", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); expect(appDb.queryAll<{ companyId: string }>("select company_id as companyId from secrets where id = 'secret-1'")[0]) .toEqual({ companyId: "company-1" }); @@ -208,6 +209,7 @@ describe("legacy to v56 baseline migration", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); expect(appDb.queryAll("select id from budget_policies where scope_type = 'agent' and scope_id = 'agent-1'")) .toHaveLength(1); @@ -232,6 +234,7 @@ describe("legacy to v56 baseline migration", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); } finally { appDb.close(); @@ -390,6 +393,7 @@ describe("legacy to v56 baseline migration", () => { { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); } finally { appDb.close(); diff --git a/tests/unit/requires-filesystem-migration.test.ts b/tests/unit/requires-filesystem-migration.test.ts index 67af8f54..4403d510 100644 --- a/tests/unit/requires-filesystem-migration.test.ts +++ b/tests/unit/requires-filesystem-migration.test.ts @@ -116,12 +116,13 @@ describe("legacy to v56 baseline migration", () => { { id: "task-inflight", requiresFilesystem: 1 }, ]); const migrations = appDb.queryAll<{ version: number; name: string }>( - "select version, name from schema_migrations where version in (56, 57, 58) order by version", + "select version, name from schema_migrations where version in (56, 57, 58, 59) order by version", ); expect(migrations).toEqual([ { version: 56, name: "baseline_v56" }, { version: 57, name: "task_leases_runtime_ownership" }, { version: 58, name: "approval_policy_table" }, + { version: 59, name: "social_company_scope" }, ]); } finally { appDb.close();