-
Notifications
You must be signed in to change notification settings - Fork 20
feat(server): device workers can claim runs in orgs they're pinned to #1149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
de6da7f
feat(server): device workers can claim runs in orgs they're pinned to
buremba e387029
fix(server): authorize run completion/heartbeat for watcher-pinned de…
buremba b9b719b
fix(server): unpinned capability runs use base org scope, not widened
buremba 6c7f2f8
fix(server): device manual watcher trigger honors cross-org pins
buremba 7fa572d
fix(server): drop duplicate workerUserId decl in manual trigger
buremba File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
108 changes: 108 additions & 0 deletions
108
packages/server/src/__tests__/integration/auth/device-cross-org-pins.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /** | ||
| * resolveDeviceClaimableOrgs — cross-org device pin scoping. | ||
| * | ||
| * A user-scoped device worker's base scope is [token's bound org, personal org]. | ||
| * This helper widens it to orgs where the device has an active pin AND its owner | ||
| * is still a member. Pinning is the owner's consent (see watcher-device-access); | ||
| * the membership join revokes scope when the owner leaves the org. | ||
| */ | ||
| import { beforeEach, describe, expect, it } from 'vitest'; | ||
| import { resolveDeviceClaimableOrgs } from '../../../utils/device-claimable-orgs'; | ||
| import { getNextNumericId } from '../../../tools/admin/helpers/db-helpers'; | ||
| import { cleanupTestDatabase, getTestDb } from '../../setup/test-db'; | ||
| import { | ||
| addUserToOrganization, | ||
| createTestAgent, | ||
| createTestOrganization, | ||
| createTestUser, | ||
| } from '../../setup/test-fixtures'; | ||
|
|
||
| const sql = getTestDb(); | ||
|
|
||
| async function insertDevice(userId: string, orgId: string): Promise<string> { | ||
| const workerId = `mac-${Math.random().toString(36).slice(2, 10)}`; | ||
| const rows = (await sql` | ||
| INSERT INTO device_workers (user_id, worker_id, platform, capabilities, label, organization_id) | ||
| VALUES (${userId}, ${workerId}, 'macos', ${sql.json({})}, 'Test Mac', ${orgId}) | ||
| RETURNING id | ||
| `) as unknown as Array<{ id: string }>; | ||
| return String(rows[0].id); | ||
| } | ||
|
|
||
| async function pinWatcher(opts: { | ||
| orgId: string; | ||
| agentId: string; | ||
| deviceWorkerId: string; | ||
| createdBy: string; | ||
| status?: string; | ||
| }): Promise<void> { | ||
| const id = await getNextNumericId(sql, 'watchers'); | ||
| await sql` | ||
| INSERT INTO watchers ( | ||
| id, status, created_by, organization_id, agent_id, watcher_group_id, | ||
| notification_channel, notification_priority, min_cooldown_seconds, | ||
| device_worker_id, slug, created_at, updated_at | ||
| ) VALUES ( | ||
| ${id}, ${opts.status ?? 'active'}, ${opts.createdBy}, ${opts.orgId}, ${opts.agentId}, ${id}, | ||
| 'notification', 'normal', 300, ${opts.deviceWorkerId}, ${`w-${id}`}, NOW(), NOW() | ||
| ) | ||
| `; | ||
| } | ||
|
|
||
| describe('resolveDeviceClaimableOrgs (cross-org device pins)', () => { | ||
| beforeEach(async () => { | ||
| await cleanupTestDatabase(); | ||
| }); | ||
|
|
||
| it('adds an org with an active pin when the owner is a member, and excludes a non-member org', async () => { | ||
| const user = await createTestUser(); | ||
| const orgA = await createTestOrganization(); // base scope (bound/personal) | ||
| const orgB = await createTestOrganization(); // member + pinned -> included | ||
| const orgC = await createTestOrganization(); // pinned but NOT a member -> excluded | ||
| await addUserToOrganization(user.id, orgA.id, 'owner'); | ||
| await addUserToOrganization(user.id, orgB.id, 'owner'); | ||
| // intentionally NOT a member of orgC | ||
|
|
||
| const deviceWorkerId = await insertDevice(user.id, orgA.id); | ||
| const agentB = await createTestAgent({ organizationId: orgB.id, ownerUserId: user.id }); | ||
| const agentC = await createTestAgent({ organizationId: orgC.id, ownerUserId: user.id }); | ||
| await pinWatcher({ orgId: orgB.id, agentId: agentB.agentId, deviceWorkerId, createdBy: user.id }); | ||
| await pinWatcher({ orgId: orgC.id, agentId: agentC.agentId, deviceWorkerId, createdBy: user.id }); | ||
|
|
||
| const result = await resolveDeviceClaimableOrgs(sql, { | ||
| deviceWorkerId, | ||
| ownerUserId: user.id, | ||
| baseOrgIds: [orgA.id], | ||
| }); | ||
|
|
||
| expect(result).toContain(orgA.id); // base scope always present | ||
| expect(result).toContain(orgB.id); // pinned + member | ||
| expect(result).not.toContain(orgC.id); // pinned but not a member | ||
| }); | ||
|
|
||
| it('does not grant scope from an archived watcher pin', async () => { | ||
| const user = await createTestUser(); | ||
| const orgA = await createTestOrganization(); | ||
| const orgB = await createTestOrganization(); | ||
| await addUserToOrganization(user.id, orgA.id, 'owner'); | ||
| await addUserToOrganization(user.id, orgB.id, 'owner'); | ||
|
|
||
| const deviceWorkerId = await insertDevice(user.id, orgA.id); | ||
| const agentB = await createTestAgent({ organizationId: orgB.id, ownerUserId: user.id }); | ||
| await pinWatcher({ | ||
| orgId: orgB.id, | ||
| agentId: agentB.agentId, | ||
| deviceWorkerId, | ||
| createdBy: user.id, | ||
| status: 'archived', | ||
| }); | ||
|
|
||
| const result = await resolveDeviceClaimableOrgs(sql, { | ||
| deviceWorkerId, | ||
| ownerUserId: user.id, | ||
| baseOrgIds: [orgA.id], | ||
| }); | ||
|
|
||
| expect(result).toEqual([orgA.id]); // archived pin grants nothing | ||
| }); | ||
| }); |
43 changes: 43 additions & 0 deletions
43
packages/server/src/__tests__/unit/run-in-worker-scope.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { runInWorkerScope } from '../../utils/device-claimable-orgs'; | ||
|
|
||
| const base = { organization_id: 'orgB', device_owner: null, watcher_device_owner: null }; | ||
|
|
||
| describe('runInWorkerScope', () => { | ||
| it('in scope when the run org is in the base scope', () => { | ||
| expect(runInWorkerScope(base, { workerUserId: 'u1', orgIds: ['orgB'] })).toBe(true); | ||
| }); | ||
|
|
||
| it('in scope when the worker owns the run\'s pinned connection device', () => { | ||
| expect( | ||
| runInWorkerScope( | ||
| { ...base, device_owner: 'u1' }, | ||
| { workerUserId: 'u1', orgIds: ['orgA'] } | ||
| ) | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('in scope when the worker owns the run\'s pinned watcher device (cross-org)', () => { | ||
| expect( | ||
| runInWorkerScope( | ||
| { ...base, watcher_device_owner: 'u1' }, | ||
| { workerUserId: 'u1', orgIds: ['orgA'] } | ||
| ) | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('forbidden when org is out of scope and the worker owns neither pinned device', () => { | ||
| expect( | ||
| runInWorkerScope( | ||
| { ...base, device_owner: 'someone-else', watcher_device_owner: 'someone-else' }, | ||
| { workerUserId: 'u1', orgIds: ['orgA'] } | ||
| ) | ||
| ).toBe(false); | ||
| }); | ||
|
|
||
| it('forbidden when there is no worker user and the org is out of scope', () => { | ||
| expect( | ||
| runInWorkerScope({ ...base, watcher_device_owner: 'u1' }, { workerUserId: null, orgIds: [] }) | ||
| ).toBe(false); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /** | ||
| * Resolve which orgs a user-scoped device worker may claim runs in. | ||
| * | ||
| * Base scope (computed in the /api/workers/* auth middleware) is the token's | ||
| * bound org plus the user's personal org. On top of that, a device may claim | ||
| * runs in any org where it has a pinned watcher/connection AND its owner is | ||
| * still a member of that org. | ||
| * | ||
| * The pin IS the consent: `evaluateDeviceWorkerAccess` only lets a device's | ||
| * owner attach it to a resource, so a pin in org B means the owner explicitly | ||
| * opted this device into serving org B. The membership join means access is | ||
| * revoked automatically if the owner later leaves the org. Within-org claiming | ||
| * still follows the pinned/capability rules in the poll, so the device only | ||
| * ever runs the resource it was actually pinned to. | ||
| * | ||
| * Only `active` watchers and non-deleted connections count — an archived | ||
| * watcher or deleted connection must not keep an org in scope. | ||
| */ | ||
| import type { DbClient } from '../db/client'; | ||
|
|
||
| export async function resolveDeviceClaimableOrgs( | ||
| sql: DbClient, | ||
| params: { deviceWorkerId: string; ownerUserId: string; baseOrgIds: string[] } | ||
| ): Promise<string[]> { | ||
| const rows = (await sql` | ||
| SELECT DISTINCT src.organization_id | ||
| FROM ( | ||
| SELECT organization_id FROM watchers | ||
| WHERE device_worker_id = ${params.deviceWorkerId} AND status = 'active' | ||
| UNION | ||
| SELECT organization_id FROM connections | ||
| WHERE device_worker_id = ${params.deviceWorkerId} AND deleted_at IS NULL | ||
| ) src | ||
| JOIN "member" m | ||
| ON m."organizationId" = src.organization_id AND m."userId" = ${params.ownerUserId} | ||
| WHERE src.organization_id IS NOT NULL | ||
| `) as unknown as Array<{ organization_id: string }>; | ||
|
|
||
| const pinnedOrgIds = rows | ||
| .map((r) => r.organization_id) | ||
| .filter((id): id is string => typeof id === 'string' && id.length > 0); | ||
|
|
||
| return Array.from(new Set([...params.baseOrgIds, ...pinnedOrgIds])); | ||
| } | ||
|
|
||
| /** | ||
| * Whether a user-scoped device worker may act on a run (claim / complete / | ||
| * heartbeat). True when the run's org is in the worker's base scope, OR the | ||
| * worker's user owns the device the run is pinned to — via either a pinned | ||
| * connection (`device_owner`) or a pinned watcher (`watcher_device_owner`). | ||
| * Pinning is the owner's consent, so a device may finish a run it was attached | ||
| * to in any org, mirroring the claim-side scope. | ||
| */ | ||
| export function runInWorkerScope( | ||
| run: { | ||
| organization_id: string; | ||
| device_owner: string | null; | ||
| watcher_device_owner: string | null; | ||
| }, | ||
| ctx: { workerUserId: string | null; orgIds: string[] } | ||
| ): boolean { | ||
| if (ctx.orgIds.includes(run.organization_id)) return true; | ||
| if (!ctx.workerUserId) return false; | ||
| return ( | ||
| run.device_owner === ctx.workerUserId || run.watcher_device_owner === ctx.workerUserId | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reapply org-membership checks for owned cross-org runs.
This helper now treats
device_owner/watcher_device_owneras sufficient on their own, butresolveDeviceClaimableOrgs()only grants cross-org scope while the owner is still a member. Once membership is revoked,authorizeRunForWorker()will still allow heartbeat/completion for already-running cross-org runs because this path ignores the resolved claimable-org set entirely.[suggested fix: have completion/heartbeat authorization resolve the worker's current claimable orgs and only allow the ownership fallback when
run.organization_idis still in that set.]🤖 Prompt for AI Agents