feat(agents-server): add permission enforcement#4475
Conversation
b79dbd1 to
8d5b80c
Compare
Electric Agents Mobile BuildAndroid preview build for commit
|
8d5b80c to
6367362
Compare
|
I read through this against the main sharing flows we want to support in Desktop/Mobile:
Overall this is a strong foundation: private-by-default via 1. Spawn-time grants need authority validationThe issue explicitly calls out validating spawn-time grants so the spawner cannot grant beyond their authority. Right now for (const grant of parsed.grants ?? []) {
await ctx.entityManager.registry.createEntityPermissionGrant({
entityUrl: entity.url,
permission: grant.permission,
subjectKind: grant.subject_kind,
subjectValue: grant.subject_value,
propagation: grant.propagation,
copyToChildren: grant.copy_to_children,
expiresAt: parseExpiresAt(grant.expires_at),
createdBy: principal.url,
})
}So a caller with type-level {
"subject_kind": "principal_kind",
"subject_value": "user",
"permission": "manage",
"propagation": "descendants",
"copy_to_children": true
}If the intended rule is “the spawner owns the newly created entity and therefore may grant any permission on it”, that should be documented and tested explicitly. But I think we likely want a validation step, especially for:
At minimum, broad/propagating grants should require some explicit grant authority, e.g. 2. Electric entity visibility should match server-side
|
6367362 to
be8e3f9
Compare
…nfig (#4495) ## What Started as a fix for three CI failures observed on `main`; reviving the TS suite (the first fix) then surfaced pre-existing typecheck breakage that had been hidden while CI was dark, which this PR also cleans up. ### CI / build fixes | Job | Verdict | Fix | |-----|---------|-----| | **TS tests** | 🔴 Real — startup-failing on *every* run since #4450 | Job-scoped `packages: write` on the `ensure_sync_service_image` caller | | **Agents Desktop Canary** | 🔴 Real — fails every run since #4441 | `-c.channel` → `-c.publish.channel` | | **Changesets** | 🟡 Flake — intermittent `agents-runtime` dts race | Promote `skills/types` to a tsdown entry | ### Typecheck fixes (pre-existing breakage exposed by re-enabling TS CI) | File | Issue | Fix | |------|-------|-----| | `agents-runtime/src/pi-adapter.ts` | merged assistant `content` typed `unknown[]` (#4449) | annotate the block-array type | | `agents-runtime/src/webhook-signature.ts` | `node:crypto` no longer exports `JsonWebKey` | cast input to `JsonWebKeyInput` | | `agents-runtime/src/sandbox/docker.ts` | `isDockerAvailable` not on the subpath the tsconfig wildcard resolves | re-export it (cascade fix for `electric-ax` / conformance) | | `agents-server-ui/tsconfig.json` | UI typechecked agents-runtime's node-only sandbox source via `paths` | resolve the index via built `.d.ts`; keep only the browser-safe `client` source-mapped — UI stays node-free | ## Why (CI fixes) - **TS tests:** #4450 downgraded the workflow's top-level token to `packages: read`, but `ts_tests.yml` is the sole caller of the reusable `ensure_sync_service_image.yml`, whose job requests `packages: write` to push the sync-service image to GHCR. A called reusable workflow cannot elevate permissions above the caller's token, so GitHub failed the **entire run at startup** — meaning the TS test suite had not run on any commit (main or PR) since. Fixed by granting `packages: write` only on the caller job, keeping the top-level token at `read` per #4450's hardening. - **Agents Desktop Canary:** electron-builder 26.8.1 rejects the config with `unknown property 'channel'`. `channel` is not a valid root property — moved under the publish provider (`-c.publish.channel=beta`, alongside the existing `-c.publish.url`). - **Changesets:** `agents-runtime` dts build intermittently fails with `UNLOADABLE_DEPENDENCY: Could not load src/skills/types.d.ts` under CI's parallel build. Promoting `src/skills/types.ts` to a first-class tsdown entry makes its `.d.ts` a stable named output instead of a raced chunk. ## Validation - Workflow files parse as valid YAML. - All previously-failing typecheck packages (`agents-runtime`, `electric-ax`, `agents-server-ui`, `agents-server-conformance-tests`) verified green via CI-faithful isolated (`--frozen-lockfile`) install + build + typecheck. ## Not in scope (pre-existing, flagged separately) - `runtime-dsl.test.ts` (92 tests, `401 UNAUTHORIZED: Principal is not allowed to spawn`) — from #4475's permission enforcement (@icehaunter); test fixtures need spawn permission seeded. Not a build/type issue. - An `agents-mcp` dts-race flake in the conformance build (same class as the `skills/types` one). ## Note for reviewer The canary maps channel input `canary` → publish channel `beta`. Preserved the existing value, but flagging in case it should be `canary`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…registry changes After rebasing on top of main, two main-side changes that landed since this branch's base broke a handful of tests that were green at branch time. Aligning the mocks so CI doesn't false-alarm on this PR. 1. `add permission enforcement` (#4475) made the spawn route 401 for any non-bypass principal that doesn't hold the required entity-type grant. The runtime-dsl test framework was using `system:runtime-dsl-test`, which isn't a built-in bypass — switch to `system:dev-local` (the same bypass principal the desktop's local runtime uses). 70 snapshots auto-regenerated; the diff is purely the `from` field, nothing semantic. The dispatch-policy- routing test setup gets the same swap, plus its mocked runner's `owner_principal` flipped to match (so the `assertDispatchPolicyAllowed` owner check passes). 2. A new `registry.replaceSharedStateLink` call landed in `syncManifestLinks` without updating the registry mocks in agents-server's status / write-validation / server-start tests. Added `replaceSharedStateLink: vi.fn()` (or a no-op method on the fake registry class) to each. None of these tests are exercised by the recent main commits' CI matrix (TS tests are scoped by affected workspace, and main has only touched agents-desktop / agents-mobile recently), which is why they appear to "pass on main" but red on this PR's CI. The fixes above keep them passing and let our actual change get a clean signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ute tests Same pattern as the previous rebase-fixup commit: another test file landed on main with a non-bypass principal (`system:test`), and the new permission-enforcement code (#4475) now reaches for `registry.hasEntityPermission` via canAccessEntity — which the test's mock registry doesn't expose. Switching the principal to `system:dev-local` (a built-in bypass) sidesteps the entity-permission check, same way the desktop's local runtime and the dispatch-policy-routing tests do. These tests assert subscription routing, not authz, so the bypass is the minimal fix and matches the convention established in the previous commit on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…registry changes After rebasing on top of main, two main-side changes that landed since this branch's base broke a handful of tests that were green at branch time. Aligning the mocks so CI doesn't false-alarm on this PR. 1. `add permission enforcement` (#4475) made the spawn route 401 for any non-bypass principal that doesn't hold the required entity-type grant. The runtime-dsl test framework was using `system:runtime-dsl-test`, which isn't a built-in bypass — switch to `system:dev-local` (the same bypass principal the desktop's local runtime uses). 70 snapshots auto-regenerated; the diff is purely the `from` field, nothing semantic. The dispatch-policy- routing test setup gets the same swap, plus its mocked runner's `owner_principal` flipped to match (so the `assertDispatchPolicyAllowed` owner check passes). 2. A new `registry.replaceSharedStateLink` call landed in `syncManifestLinks` without updating the registry mocks in agents-server's status / write-validation / server-start tests. Added `replaceSharedStateLink: vi.fn()` (or a no-op method on the fake registry class) to each. None of these tests are exercised by the recent main commits' CI matrix (TS tests are scoped by affected workspace, and main has only touched agents-desktop / agents-mobile recently), which is why they appear to "pass on main" but red on this PR's CI. The fixes above keep them passing and let our actual change get a clean signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ute tests Same pattern as the previous rebase-fixup commit: another test file landed on main with a non-bypass principal (`system:test`), and the new permission-enforcement code (#4475) now reaches for `registry.hasEntityPermission` via canAccessEntity — which the test's mock registry doesn't expose. Switching the principal to `system:dev-local` (a built-in bypass) sidesteps the entity-permission check, same way the desktop's local runtime and the dispatch-policy-routing tests do. These tests assert subscription routing, not authz, so the bypass is the minimal fix and matches the convention established in the previous commit on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Motivation
Agents Server needs authorization at every entity access path so tenants can limit which principals can spawn types and access individual entity instances. The default model should keep entity instances owner-private while still allowing common sharing flows through explicit grants, principal-kind grants, spawn-time inheritance, and Electric-visible effective permissions.
fixes #4464
What changed
registerTypes().spawngrants forprincipal_kind=useron Horton and Worker.entity_effective_permissionsfor point decisions and Electric visibility.entities(tags)observation streams by the authenticated principal. Entitymanagegrants are included in read visibility, and entity-typemanagegrants are included in spawn/type visibility.manageon the parent before they can be delegated.packages/agents-server/docker-compose.dev.ymlwithELECTRIC_FEATURE_FLAGS=allow_subqueries.Permissions model
Principals are still defined outside Agents Server. The server consumes the authenticated principal from the request context/header and stores grants against either a concrete principal URL, such as
/principal/user%3Aalice, or a principal kind, such asuser,agent,service, orsystem. Native group management is intentionally out of scope for this version; principal-kind grants cover the common broad-access cases.Type grants control type-level actions only. A type-level
spawngrant decides whether a principal may create an instance of a registered entity type; type-levelmanageis used for type mutation and is treated as a superset for type visibility/spawn checks. Type grants never grant access to existing entity instances. New type creation is open to authenticated principals so developers can register new entity definitions, while mutation of an existing type definition remains guarded bymanage.Entity grants control access to individual entity instances. Instance access defaults to
created_byownership, plus explicit/effective grants for verbs such asread,write,delete,signal,fork,schedule, and parentspawn. Entitymanageis treated as a superset for point checks and visibility.The permission verbs are intentionally granular.
writecovers content mutation paths such as send, tags, inbox edits, attachments, and event-source subscription edits. It does not implydelete,signal,fork,schedule, parentspawn, ormanage; those require their own grants ormanage. UI sharing presets like “read-only”, “read/write”, and “full control” should expand into the appropriate set of granular grants.Spawn links the new entity into the permission graph:
spawngrant for the target type.spawnon that parent entity.created_byas the caller principal.copy_to_childrenare copied into the child as direct grants during spawn.manageon the parent. This coversprincipal_kindgrants,managegrants,propagation=descendants, andcopy_to_children=true.Key decisions
IN (SELECT ...)subqueries with no correlated outer references.created_byownership plus explicit/effective grants.subject_kind=principal_kindandsubject_value=user. This assumes an Agents Server tenant/service maps to a workspace boundary, and only workspace members can authenticate asuserprincipals in that tenant.public/linksubject kind, synthetic anonymous/link principals, or Cloud share-link records that resolve into an authorization context with matching Electric visibility behavior.now()in Electric predicates.Coverage
Added focused coverage for permission service behavior, route middleware, durable stream access, shared-state authorization, Electric SQL predicate generation, principal-scoped observation streams, runtime registration grant payloads, spawn-time grant materialization and parented-spawn delegation checks, and built-in Horton/Worker spawn grants.
A local smoke run also registered a linked
/tmpruntime, spawned an entity asuser:test, confirmeduser:otherwas denied list/send/view before grants, then confirmeduser:othercould list/send/view after explicitreadandwritegrants usingELECTRIC_AGENTS_SERVER_HEADERSwithElectric-Principal.