cloudflare / cloudflare/cloudflare-os

Admin-disabled gatekeeper/resource is still auto-provisioned via the ambient singleton path

Open
#488 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
9.9k
Forks
1.2k
Avg merge
1d 20h
Merged PRs (30d)
107

Description

### Summary
`AGENTS.md` documents `user.ts`'s `getGatekeeperClassFor()` as "the single core chokepoint where disabled gatekeepers/resources are enforced before a capability is minted." That is true for the path it covers, but the ambient (auto-supplied) singleton-capsule path never calls `getGatekeeperClassFor()` at all, and the two functions it does call perform no disabled-check whatsoever. As a result, once a user has connected an account whose `AccountDescription` declares `singleton`, disabling that gatekeeper (or a specific resource under it) from the admin panel has no effect on already-connected users: an ambient capsule for it is still installed into any of their gadgets that does not already have one, when that gadget is opened (capsules are provisioned once per gadget, so this includes every newly created gadget).

The type contract for `AccountDescription.singleton` does not require `autoProvisionsAccount: true` on the vendor. All shipped examples of `singleton` happen to be on auto-provisioning vendors, so the disabled-check that exists only for that case has apparently never been exercised for a manually OAuth-connected vendor's account before. We hit this in a manually-connected, OAuth-based MCP gatekeeper we integrated (not part of this repo) whose account declares `singleton`.

### Affected code (at pinned commit `af56a9d79d8a60ebed8dabb11b075cd88efc1b87`)

- `AGENTS.md:48` — documents `user.ts`'s `getGatekeeperClassFor()` as the single chokepoint for disabled gatekeepers/resources.
- `packages/workshop-backend/src/user.ts:1666-1690` (`getGatekeeperClassFor()`) — this is the function that actually checks `config.disabledGatekeepers` (line 1678) and `isResourceDisabled(...)` (line 1685) before minting a capability. It throws in both cases.
- `packages/workshop-backend/src/user.ts:1332-1343` (`listProvidedAccounts()`) — the only disabled-related check here is at line 1340: `if (rec.autoProvisioned && ambientGatekeeperMode(config, rec.vendorId) === "disabled") continue;`. It does not consult `config.disabledGatekeepers` or `isResourceDisabled(...)` at all. This function is what the ambient path uses to decide which accounts to surface.
- `packages/workshop-backend/src/user.ts:1352-1359` (`getSingletonGatekeeperClass(accountId)`) — checks only that `record.description.singleton` is set (line 1357), then directly calls through to the account's own `getSingletonGatekeeperClass()`. No disabled-check of any kind.
- `packages/workshop-backend/src/overseer.ts:6183-6234` (`ensureAmbientCapsules()`) — the ambient entry point, called whenever a gadget is opened (call site at `overseer.ts:8297`). It calls `listProvidedAccounts()` (line ~6189, filtering on `account.description.singleton?.tsType`), then for each candidate calls `ownerDo.getSingletonGatekeeperClass(account.accountId)` (line 6222) and installs the result as an ambient Facet via `addGatekeeper(cls, {type: "ambient", ...})` (line 6228). A per-account failure is caught and logged (line 6229) but does not stop other accounts or the caller.
- `packages/workshop-backend/src/admin-settings.ts:473-492` (`setGatekeeperMode()`) — for a vendor whose `autoProvisionsAccount` is not `true` (the case of an OAuth-connected vendor), disabling it only writes the vendor id into `config.disabledGatekeepers` (line 489-490). That is exactly the field `listProvidedAccounts()` never reads, so admin disablement for this class of vendor structurally cannot reach the ambient path.
- `packages/workshop-shared/src/gatekeeper.ts:75-77` — the JSDoc for `GatekeeperVendor.autoProvisionsAccount` states "The account — not the vendor — declares whether it provides an agent singleton"; `packages/workshop-shared/src/gatekeeper.ts:170-175` — the `AccountDescription.singleton` field itself carries no constraint tying it to `autoProvisionsAccount`. So the type contract permits (and the runtime accepts) a manually-connected, non-auto-provisioning account declaring `singleton`, which is exactly the combination that bypasses the disabled-check.

### Steps to reproduce (traced through source; not executed against a live deployment)
1. Implement a `GatekeeperVendor` that does not set `autoProvisionsAccount` (i.e., users connect it manually via OAuth), whose connected `Account`'s `describe()` returns an `AccountDescription` with `singleton` set.
2. Have a user connect that account.
3. As an admin, disable that gatekeeper (or one of its resources) from the `/admin` panel. This is implemented by `admin-settings.ts`'s `setGatekeeperMode()`, which — because `autoProvisionsAccount` is not `true` — writes only to `config.disabledGatekeepers`.
4. Have the already-connected user create and open a new gadget (a gadget that does not yet have an ambient capsule for this account).
5. Observe: `overseer.ts`'s `ensureAmbientCapsules()` still calls `listProvidedAccounts()`, which still returns this account (its only disabled-related check, `ambientGatekeeperMode(...) === "disabled"`, is gated on `rec.autoProvisioned`, which is false here), and `getSingletonGatekeeperClass()` still returns the class with no check of `config.disabledGatekeepers`/`isResourceDisabled`. A new ambient capsule for the "disabled" gatekeeper is installed.

### Expected vs. actual
- Expected: once an admin disables a gatekeeper or resource, no new capability for it should be mintable for any account, ambient or manual, matching the documented "single core chokepoint" guarantee in `AGENTS.md:48`.
- Actual: the ambient/singleton path (`listProvidedAccounts()` → `getSingletonGatekeeperClass()`) mints a new capability (installs a new ambient Facet) regardless of `disabledGatekeepers`/resource-disabled state, as long as the account is not itself `autoProvisioned` with its `ambientGatekeeperMode` set to `"disabled"`.

### Impact
For any vendor/account combination where a manually (OAuth) connected account declares `singleton` — a combination the type contract allows but no shipped vendor currently exercises — admin disablement is silently ineffective for already-connected users:
- Ambient capsules for the "disabled" gatekeeper continue to be provisioned into gadgets that do not yet have one (including every newly created gadget) when they are opened.
- Whatever the singleton gatekeeper's own read-only tool surface is continues to run without further gating (this depends on the specific gatekeeper's own tool/approval policy, not on anything in `cloudflare-os` itself).
- The admin has no `/admin`-panel-visible signal that the disablement did not take effect, since the write to `disabledGatekeepers` itself succeeds and is exactly the field the auto-provisioning path does check.

We believe this is a control-plane/availability defect (an administrator's stop control silently not applying), not a privilege-escalation vector by itself — its severity depends on what the specific singleton gatekeeper exposes.

### Suggested fix
In `packages/workshop-backend/src/user.ts`, `listProvidedAccounts()` (around lines 1332-1343) currently filters only on `rec.autoProvisioned && ambientGatekeeperMode(...) === "disabled"`. Extending that filter to also check `config.disabledGatekeepers` (as `getGatekeeperClassFor()` already does at line 1678) and per-resource disablement via `isResourceDisabled(...)` (as at line 1685) for the account's vendor would close the gap without changing the type contract. We have not implemented or tested this change against the upstream repository; we only worked around it downstream (see Notes).

### Notes
- We noticed this because a manually OAuth-connected gatekeeper we integrated declares `singleton` — a combination the type contract in `workshop-shared/src/gatekeeper.ts` permits but which, as far as we can tell from source, no existing vendor in this repository exercises (the shipped `singleton` examples are all on auto-provisioning vendors).
- As a downstream workaround (not a fix to this repository, and not proposed for upstream), we stopped declaring `singleton` on our account's `describe()`, made our `getSingletonGatekeeperClass()` implementation unconditionally throw, and separated our manual-connection path (`getGatekeeperClassFor()`) into its own internal code path so the ambient rejection does not affect manual connections. We also added a deploy-time kill switch in our own gatekeeper as defense in depth. None of this touches `cloudflare-os` itself.
- Also worth noting (lower priority, same root area): the disabled-check only runs at capability-mint time. Any ambient capsule or manually-bound capability created *before* an admin disables the gatekeeper is not retroactively revoked by that disablement. We have not investigated whether this is intentional or already tracked.
- All line numbers above were re-verified directly against the source at the pinned commit above by us; we did not simply copy them from an internal note without checking.

Contributor guide

Open the contributing guide

Research direction

Start at ensureAmbientCapsules() in packages/workshop-backend/src/overseer.ts, then trace listProvidedAccounts() and getSingletonGatekeeperClass() in user.ts; compare their behavior with getGatekeeperClassFor() and setGatekeeperMode(). Done means an admin-disabled gatekeeper or resource is not surfaced or provisioned through the ambient singleton path for connected accounts.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authorization, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.