From 30880194cfd9378791e915bfff388d323923eb21 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Fri, 24 Jul 2026 11:19:07 +0300 Subject: [PATCH 1/3] fix(cli): derive customer/billing org from active workspace, not /workspaces row[0] customers new + customers list picked the org from the cross-org /workspaces union's row[0], which for a user in 2+ orgs is nondeterministic and 403s when it disagrees with the token's org (backend can() requires path-org==token-org). Extract a shared resolveOrgPublicIdForWorkspace helper (from billing's local version, minus the row[0] fallback), use it in customers new, and scope customers list to the active org. billing refactored onto the shared helper. AIT-263 --- src/__tests__/customers.test.ts | 24 +++++++++++++++++ src/commands/__tests__/billing.test.ts | 14 +++++++--- src/commands/_helpers.ts | 27 +++++++++++++++++++ src/commands/billing.ts | 32 +++++++--------------- src/commands/customers.ts | 37 +++++++++++++++++--------- 5 files changed, 95 insertions(+), 39 deletions(-) diff --git a/src/__tests__/customers.test.ts b/src/__tests__/customers.test.ts index 0b11bd6..c6f6c3c 100644 --- a/src/__tests__/customers.test.ts +++ b/src/__tests__/customers.test.ts @@ -72,6 +72,9 @@ async function runCustomers(args: string[]): Promise { describe('customers list', () => { it('lists only customer-kind workspaces (JSON mode)', async () => { + // AIT-263: customers list is scoped to the ACTIVE org, so an active + // workspace must be resolvable. + fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); mockedApi.mockResolvedValue(fakeWorkspaces); await runCustomers(['list', '--json']); @@ -81,6 +84,7 @@ describe('customers list', () => { }); it('human mode renders customer rows only', async () => { + fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); mockedApi.mockResolvedValue(fakeWorkspaces); await runCustomers(['list']); @@ -108,6 +112,7 @@ describe('customers use', () => { describe('customers new', () => { it('creates an empty customer via POST /organizations/:orgId/customers', async () => { + fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); mockedApi.mockImplementation(async (path: string) => { if (path === '/workspaces') return fakeWorkspaces; return { id: 'ws_NEWCUST1', name: 'Fresh Client', externalId: 'crm-9' }; @@ -122,6 +127,25 @@ describe('customers new', () => { expect(logs).toContain('ws_NEWCUST1'); }); + it('AIT-263: targets the ACTIVE org, never union row[0], for a multi-org user', async () => { + // row[0] is org_pub_OTHER; the active workspace ws_TEAM0001 is org_pub_A. + // The pre-fix code POSTed to row[0] (wrong org → 403); it must use org_pub_A. + const twoOrgUnion = [ + { id: 'ws_OTHER001', name: 'Other', organizationPublicId: 'org_pub_OTHER', role: 'admin', createdAt: '2026-01-01', kind: 'team' }, + ...fakeWorkspaces, + ]; + fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); + mockedApi.mockImplementation(async (path: string) => { + if (path === '/workspaces') return twoOrgUnion; + return { id: 'ws_NEWCUST2', name: 'Bob', externalId: null }; + }); + await runCustomers(['new', 'Bob']); + + const postCall = mockedApi.mock.calls.find((c) => c[1]?.method === 'POST'); + expect(postCall?.[0]).toBe('/organizations/org_pub_A/customers'); + expect(String(postCall?.[0])).not.toContain('org_pub_OTHER'); + }); + it('does NOT switch the active workspace to the new customer', async () => { fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); mockedApi.mockImplementation(async (path: string) => { diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index af399b3..6db3177 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -5,10 +5,16 @@ vi.mock('../../api/client.js', () => ({ })); // getDefaultWorkspaceId is read off the user's local profile; stub to a fixed -// string so the test doesn't depend on profile state. -vi.mock('../_helpers.js', () => ({ - getDefaultWorkspaceId: vi.fn(async () => 'ws_test'), -})); +// string so the test doesn't depend on profile state. resolveOrgPublicIdForWorkspace +// stays REAL (via importOriginal) so it exercises the actual active-workspace +// org derivation against the mocked apiClient union (AIT-263). +vi.mock('../_helpers.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getDefaultWorkspaceId: vi.fn(async () => 'ws_test'), + }; +}); // billingManage calls `open(url)` after resolving the Billing page URL; // without this stub, it would try to launch the user's browser. diff --git a/src/commands/_helpers.ts b/src/commands/_helpers.ts index bbead9b..42b4565 100644 --- a/src/commands/_helpers.ts +++ b/src/commands/_helpers.ts @@ -119,6 +119,33 @@ export async function getDefaultWorkspaceId(): Promise { ); } +/** + * Resolve the org publicId for an org-scoped CLI action (customers, billing) + * from the ACTIVE workspace's row in the /workspaces union. + * + * Never a bare `row[0]`: the union spans every org the user belongs to, so + * row[0] is nondeterministic for a user in 2+ orgs and 403s when it disagrees + * with the token's org (the backend `can()` boundary requires + * path-org == token-org). Pass the workspaceId from getDefaultWorkspaceId() — + * that's the workspace the CLI token is scoped to, so its org is the only one + * the backend will authorize. (AIT-263) + */ +export async function resolveOrgPublicIdForWorkspace( + workspaceId: string, +): Promise { + const all = (await apiClient('/workspaces')) as Array<{ + id: string; + organizationPublicId?: string; + }>; + const orgPublicId = all.find((w) => w.id === workspaceId)?.organizationPublicId; + if (!orgPublicId) { + throw new ValidationError( + 'No organization found for your active workspace. Run: hookmyapp workspace use ', + ); + } + return orgPublicId; +} + /** * Resolve the channel a typed command should act on. Precedence (D6): * 1. explicit --channel diff --git a/src/commands/billing.ts b/src/commands/billing.ts index a0aa586..1d64ab4 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -7,31 +7,17 @@ import { ValidationError } from '../output/error.js'; import { addExamples } from '../output/help.js'; import { cliCommandPrefix } from '../output/cli-self.js'; import { getEffectiveAppUrl } from '../config/env-profiles.js'; -import { getDefaultWorkspaceId } from './_helpers.js'; +import { getDefaultWorkspaceId, resolveOrgPublicIdForWorkspace } from './_helpers.js'; // program is lazy-imported inside actions because commands/billing.ts is // loaded by index.ts during program setup — a top-level import would form a // cycle. Same pattern as other commands that need root-level opts. -/** - * Billing is org-scoped: the workspace-addressed /stripe/subscription and - * /stripe/checkout routes are retired (410 BILLING_ROUTE_MOVED), and - * /stripe/portal before them (410 BILLING_PORTAL_RETIRED). Resolve the org - * publicId from the /workspaces membership union — every row carries - * organizationPublicId (same mechanism as customers.ts) — preferring the row - * for the active workspace. - */ -async function resolveOrgPublicId(workspaceId: string): Promise { - const all = (await apiClient('/workspaces')) as Array<{ - id: string; - organizationPublicId?: string; - }>; - const orgPublicId = (all.find((w) => w.id === workspaceId) ?? all[0])?.organizationPublicId; - if (!orgPublicId) { - throw new ValidationError('No organization found for your account. Log in and try again.'); - } - return orgPublicId; -} +// Billing is org-scoped (the workspace-addressed /stripe/subscription and +// /stripe/checkout routes are retired — 410 BILLING_ROUTE_MOVED). The org is +// resolved from the active workspace's row in the /workspaces union via the +// shared resolveOrgPublicIdForWorkspace helper (AIT-263 — one derivation for +// customers + billing, never a bare row[0]). function orgBillingUrl(orgPublicId: string): string { return `${getEffectiveAppUrl()}/org/${orgPublicId}/billing`; @@ -39,7 +25,7 @@ function orgBillingUrl(orgPublicId: string): string { export async function billingManage(opts: { json?: boolean } = {}): Promise { const workspaceId = await getDefaultWorkspaceId(); - const url = orgBillingUrl(await resolveOrgPublicId(workspaceId)); + const url = orgBillingUrl(await resolveOrgPublicIdForWorkspace(workspaceId)); // --json is a machine contract: emit the URL and take NO interactive side // effect (no browser open, no human text) so agents/CI get clean JSON. @@ -54,7 +40,7 @@ export async function billingManage(opts: { json?: boolean } = {}): Promise { const workspaceId = await getDefaultWorkspaceId(); - const orgPublicId = await resolveOrgPublicId(workspaceId); + const orgPublicId = await resolveOrgPublicIdForWorkspace(workspaceId); const [sub, usage] = await Promise.all([ apiClient(`/organizations/${orgPublicId}/billing/subscription`), apiClient('/webhook/usage', { workspaceId }), @@ -113,7 +99,7 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise { - const all = (await apiClient('/workspaces')) as Workspace[]; - const customers = all.filter((w) => w.kind === 'customer'); + // Scope to the ACTIVE org: the /workspaces union spans every org the + // user belongs to, so an unscoped filter would mix other orgs' customers + // into the list (AIT-263). Derive the active org from the active + // workspace's row and show only its customers. + const workspaceId = await getDefaultWorkspaceId(); + const all = (await apiClient('/workspaces')) as WorkspaceRow[]; + const activeOrg = all.find((w) => w.id === workspaceId)?.organizationPublicId; + const customers = all.filter( + (w) => w.kind === 'customer' && w.organizationPublicId === activeOrg, + ); if (opts.json || program.opts().json) { console.log(JSON.stringify(customers.map(dropWorkosOrgId), null, 2)); return; @@ -50,14 +64,13 @@ export function registerCustomersCommand(program: Command): void { .option('--external-id ', 'Your own identifier for this customer (CRM/system id)') .option('--json', 'Output machine-readable JSON') .action(async (name: string, opts: { externalId?: string; json?: boolean }) => { - // The customers endpoint is org-scoped; resolve the org publicId from the - // membership union (every row carries organizationPublicId). Does NOT - // switch the active workspace — an empty customer has nothing to work in. - const all = (await apiClient('/workspaces')) as Array; - const orgPublicId = all[0]?.organizationPublicId; - if (!orgPublicId) { - throw new ValidationError('No organization found for your account. Log in and try again.'); - } + // The customers endpoint is org-scoped. Resolve the org from the ACTIVE + // workspace (the org the CLI token is scoped to) — never a bare union + // row[0], which for a multi-org user is nondeterministic and 403s when + // it disagrees with the token's org (AIT-263). Does NOT switch the + // active workspace — an empty customer has nothing to work in. + const workspaceId = await getDefaultWorkspaceId(); + const orgPublicId = await resolveOrgPublicIdForWorkspace(workspaceId); const created = await apiClient(`/organizations/${orgPublicId}/customers`, { method: 'POST', body: JSON.stringify({ name, ...(opts.externalId ? { externalId: opts.externalId } : {}) }), From 885274af8f69175c02d89d566f955e2b6b4d18db Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Fri, 24 Jul 2026 14:24:17 +0300 Subject: [PATCH 2/3] fix: customers list throws the actionable error when the active workspace cannot resolve an org --- src/__tests__/customers.test.ts | 6 ++++++ src/commands/customers.ts | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/src/__tests__/customers.test.ts b/src/__tests__/customers.test.ts index c6f6c3c..ae61f27 100644 --- a/src/__tests__/customers.test.ts +++ b/src/__tests__/customers.test.ts @@ -83,6 +83,12 @@ describe('customers list', () => { expect(parsed[0].kind).toBe('customer'); }); + it('AIT-263: a stale active workspace throws the actionable ValidationError instead of an empty list', async () => { + fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_GONE0001', activeWorkspaceSlug: 'gone' })); + mockedApi.mockResolvedValue(fakeWorkspaces); + await expect(runCustomers(['list', '--json'])).rejects.toThrow(/workspace use/); + }); + it('human mode renders customer rows only', async () => { fs.writeFileSync(CONFIG_PATH, JSON.stringify({ activeWorkspaceId: 'ws_TEAM0001', activeWorkspaceSlug: 'HR' })); mockedApi.mockResolvedValue(fakeWorkspaces); diff --git a/src/commands/customers.ts b/src/commands/customers.ts index e964cf9..942133a 100644 --- a/src/commands/customers.ts +++ b/src/commands/customers.ts @@ -40,6 +40,13 @@ export function registerCustomersCommand(program: Command): void { const workspaceId = await getDefaultWorkspaceId(); const all = (await apiClient('/workspaces')) as WorkspaceRow[]; const activeOrg = all.find((w) => w.id === workspaceId)?.organizationPublicId; + if (!activeOrg) { + // A stale/absent active workspace must surface actionably — never as a + // silently empty list. Same contract as resolveOrgPublicIdForWorkspace. + throw new ValidationError( + 'No organization found for your active workspace. Run: hookmyapp workspace use ', + ); + } const customers = all.filter( (w) => w.kind === 'customer' && w.organizationPublicId === activeOrg, ); From 4b6cbc9eaff4327bb256b8ce99d94f591c092d2a Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Fri, 24 Jul 2026 15:22:09 +0300 Subject: [PATCH 3/3] test: complete the api/client partial mock with setWorkspaceContext --- src/commands/__tests__/billing.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 6db3177..b1702d9 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../../api/client.js', () => ({ apiClient: vi.fn(), + setWorkspaceContext: vi.fn(), })); // getDefaultWorkspaceId is read off the user's local profile; stub to a fixed