From b0ebd95457bea5c2e5d722f4b88b5531fa1a9776 Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:21:37 +0300 Subject: [PATCH 1/2] fix: review follow-ups for 0.6.5 (#168) - agent-runtime: missing_scope denials carry `missing`; `stepUpScopes` answers the scopes a token should add, with the handler's rules (own variants, proposeScopes, the member's role on the call's site). - api: the MCP step-up 403 asks for exactly that set and is audited; createApi refuses an mcp.oauth.resource that is not /mcp; the playground guard enforces its OAuth well-known rows. - storage/api: Cloudhal's own counters move to `rate_limits` (migration 0030), out of reach of Better Auth's 60 s rateLimit prune that reset the hourly reset-mail cap. - core-ts, analytics, astro-renderer, Studio: linear trims instead of /^-+|-+$/g (`trimChar`); datasetName was quadratic on `_` runs. - cli: the slugify test no longer claims a timing difference; only describeSetup keeps the timing assertion. - ci/docs: CodeQL cancels only pull-request runs; ADR-0012 records the undici override from #159; RFC-0044 phase 1 wording, CORS note, step-up rules, phase 2 migration renumbered 0031_oauth. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/linear-trims-168.md | 10 ++ .changeset/mcp-step-up-168.md | 6 + .changeset/rate-limits-168.md | 6 + .github/workflows/codeql.yml | 7 +- apps/studio/src/lib/field-form.ts | 18 +-- apps/studio/src/lib/media.ts | 8 +- apps/studio/src/lib/workflow.ts | 16 ++- docs/adr/0012-cloudflare-deployment.md | 2 +- docs/rfcs/0044-mcp-oauth-authorization.md | 28 +++-- packages/agent-runtime/src/index.ts | 1 + packages/agent-runtime/src/policy.ts | 23 +++- packages/agent-runtime/src/step-up.ts | 57 +++++++++ packages/agent-runtime/test/policy.test.ts | 7 +- packages/agent-runtime/test/step-up.test.ts | 107 ++++++++++++++++ packages/analytics/src/wae/index.ts | 12 +- packages/analytics/test/wae-reader.test.ts | 11 ++ packages/api/src/auth/principal.ts | 2 +- packages/api/src/mcp-mount.ts | 18 ++- packages/api/src/mcp-oauth.ts | 70 +++++++---- packages/api/src/playground.ts | 9 +- packages/api/src/staff.ts | 12 +- packages/api/src/testing-mcp-oauth.ts | 118 ++++++++++++++++-- packages/api/src/testing-rate-limits.ts | 98 +++++++++++++++ packages/api/src/tokens.ts | 4 +- packages/api/test/playground.test.ts | 27 ++++ packages/api/test/rate-limits.test.ts | 34 +++++ packages/astro-renderer/src/markdown.ts | 20 ++- packages/astro-renderer/test/kit.test.ts | 6 + packages/cli/test/studio-selfhost.test.ts | 11 +- packages/core-ts/src/media/types.ts | 9 +- packages/core-ts/src/names.ts | 26 ++-- packages/core-ts/test/media.test.ts | 4 + packages/core-ts/test/names.test.ts | 28 +++++ .../storage/migrations/0030_rate_limits.ts | 24 ++++ packages/storage/migrations/index.ts | 2 + packages/storage/test/migrate.test.ts | 6 + 36 files changed, 742 insertions(+), 105 deletions(-) create mode 100644 .changeset/linear-trims-168.md create mode 100644 .changeset/mcp-step-up-168.md create mode 100644 .changeset/rate-limits-168.md create mode 100644 packages/agent-runtime/src/step-up.ts create mode 100644 packages/agent-runtime/test/step-up.test.ts create mode 100644 packages/api/src/testing-rate-limits.ts create mode 100644 packages/api/test/rate-limits.test.ts create mode 100644 packages/storage/migrations/0030_rate_limits.ts diff --git a/.changeset/linear-trims-168.md b/.changeset/linear-trims-168.md new file mode 100644 index 00000000..d2871dab --- /dev/null +++ b/.changeset/linear-trims-168.md @@ -0,0 +1,10 @@ +--- +"@cloudhal/core-ts": patch +"@cloudhal/analytics": patch +"@cloudhal/astro-renderer": patch +"@cloudhal/cli": patch +--- + +Slug and name trims run in one linear pass instead of `/^-+|-+$/g` (CodeQL js/polynomial-redos, #168). This covers `slugify`, `foldName` and `slugifyFilename` in core-ts (the new `trimChar` is exported from `@cloudhal/core-ts/names`), heading ids in astro-renderer, and Studio's option, media and entry slugs. Results are unchanged. These trims were already linear in practice, because a run of other characters was collapsed to one `-` first. `datasetName` in analytics was quadratic on a long run of `_` inside a deployment name; it is linear now too. + +Also in 0.6.3, with no changeset at the time (#161): Studio's "Not published" badges and its "unknown" scheduler state show their status dot again. They used a Badge variant without a dot colour and rendered no dot. diff --git a/.changeset/mcp-step-up-168.md b/.changeset/mcp-step-up-168.md new file mode 100644 index 00000000..0ef66c08 --- /dev/null +++ b/.changeset/mcp-step-up-168.md @@ -0,0 +1,6 @@ +--- +"@cloudhal/agent-runtime": patch +"@cloudhal/api": patch +--- + +MCP OAuth step-up follow-ups (RFC-0044 phase 1, #168). These stay latent until phase 2 gives connections a `grantId`. A policy denial for missing scopes now carries `missing`, the required scopes that neither the caller's scopes nor an own variant satisfy. The new `stepUpScopes(policy, principal, tool, args)` turns that into the scopes a token should ask for, using the same rules as the handler: a propose call's `proposeScopes`, the own variant the member's role holds (`content:write:own` for an Author, `content:draft:own` for a Contributor, `site:publish:own` for an Author's publish) and the member's role on the call's site (RFC-0034 A2), not their installation role. The `/mcp` step-up 403 asks for exactly that set and now leaves an audit row, like other denials. `createApi` throws when `mcp.oauth.resource` is not the public URL of `/mcp`. A playground refuses both OAuth protected-resource documents (`403 {error: "playground", block: "mcp"}`), as its route table already said. diff --git a/.changeset/rate-limits-168.md b/.changeset/rate-limits-168.md new file mode 100644 index 00000000..6e42e4fc --- /dev/null +++ b/.changeset/rate-limits-168.md @@ -0,0 +1,6 @@ +--- +"@cloudhal/storage": patch +"@cloudhal/api": patch +--- + +Cloudhal's own rate-limit counters move out of Better Auth's `rateLimit` table into `rate_limits` (migration `0030_rate_limits`, #168). This covers the hourly cap of 3 reset mails per address, the per-minute buckets of the public token endpoints and each agent key's window. Better Auth prunes `rateLimit` by its own 60 s window, so the hourly reset-mail cap used to restart about a minute after any sign-in from the same address. The migration deletes the old `cloudhal:` rows from `rateLimit`, so each counter starts over once at upgrade. diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 88561a51..755bd1aa 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -21,9 +21,12 @@ on: permissions: {} +# A new push to a pull request cancels that pull request's running analysis. A push to main and +# the weekly run each get a group of their own (the run id): a concurrency group also cancels a +# *pending* run when another one queues, so sharing `refs/heads/main` would drop analyses of main. concurrency: - group: codeql-${{ github.ref }} - cancel-in-progress: true + group: codeql-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: analyze: diff --git a/apps/studio/src/lib/field-form.ts b/apps/studio/src/lib/field-form.ts index 08591405..e14017ff 100644 --- a/apps/studio/src/lib/field-form.ts +++ b/apps/studio/src/lib/field-form.ts @@ -5,6 +5,7 @@ */ import type { FieldDefinition } from "@cloudhal/core-contracts"; import { FIELD_LIMITS } from "@cloudhal/core-ts/fields"; +import { trimChar } from "@cloudhal/core-ts/names"; import { type FieldType, fieldKeyFrom, @@ -120,14 +121,15 @@ export function withOptionLabel( if (j !== i) return o; const value = isDefault && !o.valueEdited - ? label - .toLowerCase() - .normalize("NFKD") - .replace(/[\u0300-\u036f]/g, "") - .replace(/ı/g, "i") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 128) + ? trimChar( + label + .toLowerCase() + .normalize("NFKD") + .replace(/[\u0300-\u036f]/g, "") + .replace(/ı/g, "i") + .replace(/[^a-z0-9]+/g, "-"), + "-", + ).slice(0, 128) : o.value; return { ...o, value, labels: { ...o.labels, [locale]: label } }; }); diff --git a/apps/studio/src/lib/media.ts b/apps/studio/src/lib/media.ts index 8ae148e3..2e58cdc0 100644 --- a/apps/studio/src/lib/media.ts +++ b/apps/studio/src/lib/media.ts @@ -15,6 +15,7 @@ import { normalizeMediaType, RASTER_TYPES, } from "@cloudhal/core-ts/media"; +import { trimChar } from "@cloudhal/core-ts/names"; import { numberFormat } from "@cloudhal/messages/format"; import { type AssetListItem, type AssetPatch, api } from "./api.ts"; import { type ImageRef, previewImageUrl } from "./seo.ts"; @@ -389,12 +390,11 @@ export function assetPatch(form: AssetForm, kind: AssetKind): AssetPatch { /** A URL slug from a file name (lower-case words joined by hyphens), for the slug field. */ export function slugify(input: string): string { - return input + const dashed = input .toLowerCase() .replace(/ı/g, "i") .normalize("NFKD") .replace(/[\u0300-\u036f]/g, "") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, ASSET_LIMITS.slug); + .replace(/[^a-z0-9]+/g, "-"); + return trimChar(dashed, "-").slice(0, ASSET_LIMITS.slug); } diff --git a/apps/studio/src/lib/workflow.ts b/apps/studio/src/lib/workflow.ts index 8c196ce8..78d5a167 100644 --- a/apps/studio/src/lib/workflow.ts +++ b/apps/studio/src/lib/workflow.ts @@ -1,4 +1,5 @@ import type { PublishStatus } from "@cloudhal/core-contracts"; +import { trimChar } from "@cloudhal/core-ts/names"; /** Allowed editorial transitions (RFC-0001 lifecycle). */ export const TRANSITIONS: Record = { @@ -18,13 +19,10 @@ export function needsApproval(to: PublishStatus): boolean { } export function slugify(input: string): string { - return ( - input - .toLowerCase() - .normalize("NFKD") - .replace(/[̀-ͯ]/g, "") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 128) || "untitled" - ); + const dashed = input + .toLowerCase() + .normalize("NFKD") + .replace(/[̀-ͯ]/g, "") + .replace(/[^a-z0-9]+/g, "-"); + return trimChar(dashed, "-").slice(0, 128) || "untitled"; } diff --git a/docs/adr/0012-cloudflare-deployment.md b/docs/adr/0012-cloudflare-deployment.md index a4a169d3..a3bfe8d7 100644 --- a/docs/adr/0012-cloudflare-deployment.md +++ b/docs/adr/0012-cloudflare-deployment.md @@ -50,6 +50,6 @@ Decision 6 pins the direct `miniflare` devDependency (the D1, R2 and Durable Obj - **sharp.** Every Miniflare release pins sharp exactly. 4.20260730.0, still Miniflare's npm `latest`, pins 0.35.2, and the 5.20261006.0 alpha pins 0.35.4. Both are below 0.35.5, the fix for GHSA-wq5f-xc86-pv6w (librsvg), and 0.35.2 is also below 0.35.4, the fix for GHSA-rgj7-g3m4-5g8c (libheif). Miniflare loads sharp only for its Images binding and its `cf.image` fetch mock. Cloudhal configures no Images binding and makes no `cf.image` fetches: the `/cdn-cgi/image/` URLs from `@cloudhal/astro-renderer` are resolved at the edge. `pnpm-workspace.yaml` therefore overrides `sharp@<0.35.5` to exactly 0.35.5. That version was already in the lockfile through Astro, and the calls Miniflare makes behave the same on 0.35.2, 0.35.4 and 0.35.5 (checked locally on 2026-10-25). Overrides are not published. A project that uses `@cloudhal/storage/testing` with Miniflare 4 keeps Miniflare's sharp unless it adds the same override. A new project resolves the starter's `wrangler ^4.148.0` to a release whose Miniflare pins 0.35.5. Remove the override when `pnpm why -r sharp` shows nothing below 0.35.5 without it. - **wrangler** stays at 4.148.0 in this change. 4.152 is a minor move: it changes the Miniflare under `createTestHarness` (5.20261019.2-beta), needs `@cloudflare/vite-plugin` 1.65 and `@cloudflare/workers-types` ≥ 5.20261019.1, and uploads source maps on deploy by default (4.150). It gets its own change, verified with the CLI's Cloudflare suites. (wrangler 4.152.0 landed 2026-11-08; wrangler 4.158.0 and @astrojs/cloudflare 14.4.4 follow together, because 14.4 resolves @cloudflare/vite-plugin 1.67, whose peer range is wrangler ^4.156.1.) -- **undici.** The undici versions Miniflare pins (7.28.0 in 4.20260730.0, 7.29.1 in the 5.x alpha) are accepted for local development and tests: Miniflare uses undici to reach its own workerd on localhost and, outside tests, to fetch Cloudflare's `cf.json` from workers.cloudflare.com, a trusted endpoint (set `CLOUDFLARE_CF_FETCH_ENABLED=false` to turn that off). The fixed 7.30.1 is a minor move inside `dispatchFetch`, which the D1 harness and Miniflare's binding proxy depend on (the Durable Object harness bypasses it since #116), so an override waits for its own serial test run. +- **undici.** The undici versions Miniflare pins (7.28.0 in 4.20260730.0, 7.29.1 in the 5.x alpha) are accepted for local development and tests: Miniflare uses undici to reach its own workerd on localhost and, outside tests, to fetch Cloudflare's `cf.json` from workers.cloudflare.com, a trusted endpoint (set `CLOUDFLARE_CF_FETCH_ENABLED=false` to turn that off). The fixed 7.30.1 is a minor move inside `dispatchFetch`, which the D1 harness and Miniflare's binding proxy depend on (the Durable Object harness bypasses it since #116), so an override waits for its own serial test run. *Update 2026-11-08:* that run happened. #159 (Dependabot alerts #5–#32) added the override `"undici@<7.30.1": 7.30.2` to `pnpm-workspace.yaml`, so Miniflare's pinned 7.28.0 resolves to 7.30.2 in monorepo installs; the D1 and Durable Object harnesses passed on it. Like the sharp override it is not published, and it goes when Miniflare 4 is no longer pinned. Sources: https://github.com/advisories/GHSA-wq5f-xc86-pv6w · https://github.com/advisories/GHSA-rgj7-g3m4-5g8c · https://pnpm.io/settings#overrides · `npm view miniflare dist-tags`, `npm view sharp time`, `npm view wrangler time` (checked 2026-10-25) · Miniflare 4.20260730.0 `dist/src/index.js` (`imagesLocalFetcher`, `cfImageLocalFetcher`, `setupCf`). diff --git a/docs/rfcs/0044-mcp-oauth-authorization.md b/docs/rfcs/0044-mcp-oauth-authorization.md index 776827f6..60fb7ad3 100644 --- a/docs/rfcs/0044-mcp-oauth-authorization.md +++ b/docs/rfcs/0044-mcp-oauth-authorization.md @@ -100,6 +100,13 @@ not apply, cacheable for an hour: - `/.well-known/oauth-protected-resource` (root form, the fallback the 2025-11-25 and later revisions require clients to probe; Claude's docs probe it too). +Both are served without CORS headers. That matches `/mcp`, which refuses any request whose +`Origin` is not its own (DNS-rebinding protection): the clients this RFC serves are native apps +and servers, which do not need CORS, and a browser page on another origin that could read the +document still could not call `/mcp`. `McpMountOptions.oauth.resource` must be the public URL of +the mounted `/mcp` (its path ends in `/mcp`; a proxy may add a prefix): `createApi` throws +otherwise, since every token's audience is bound to it. + ```json { "resource": "https://studio.example.com/mcp", @@ -150,9 +157,14 @@ WWW-Authenticate: Bearer error="insufficient_scope", scope="site:publish", ``` This is possible without a second policy engine because each tool's scopes are static -(`toolPolicies(defineTools())`, ADR-0011): the route reads the JSON-RPC method and tool name of -the single message a stateless request carries and compares the tool's scopes with the token's. -Every other denial (role, mode, `:own`, approvals) stays a JSON-RPC result as today. Clients then +(`toolPolicies(defineTools())`, ADR-0011): the route reads the JSON-RPC method, tool name and +arguments of the single message a stateless request carries and asks the policy engine +(`stepUpScopes` in `@cloudhal/agent-runtime`) which scopes the denial lacks. The answer follows +the same rules as the handler's decision: a propose call needs its `proposeScopes`, an own +variant satisfies a tool that allows one (an Author is asked for `site:publish:own`, a +Contributor for `content:draft:own`), and the member's role is their role on the call's site +(RFC-0034 A2), not their installation role. Every other denial (role, mode, `:own`, approvals) +stays a JSON-RPC result as today. The 403 leaves an audit row like every other denial. Clients then re-authorize with the union of their scopes (the SDK's `computeScopeUnion`; the draft revision makes the union normative). A scope the role does not hold is never challenged: the client would loop. The challenge is emitted together for all scopes the tool needs ("servers SHOULD include all @@ -409,7 +421,7 @@ attestation, per-grant IP allow-lists. `better-call` 1.4.5), because the three plugins peer-depend on `^1.7.18`; `test/better-auth-pins.test.ts` keeps the family consistent. New catalog entries: `@better-auth/mcp`, `@better-auth/oauth-provider`, `@better-auth/cimd`, `jose`. -- **`packages/storage/migrations/0030_oauth.ts`**: the DDL for `oauthClient`, +- **`packages/storage/migrations/0031_oauth.ts`**: the DDL for `oauthClient`, `oauthAccessToken`, `oauthRefreshToken`, `oauthConsent`, `oauthClientAssertion`, `oauthResource`, `oauthClientResource` and `jwks`, generated with `getMigrations(auth.options).compileMigrations()` on 1.7.18 and checked in, as `0002_auth` and @@ -470,7 +482,7 @@ CREATE INDEX agent_grants_user ON agent_grants (user_id); | Phase | Ships | Amends | |---|---|---| | 1 · Discovery | `WWW-Authenticate` with `resource_metadata` and `scope` on `/mcp` 401; both PRM documents; the 403 step-up challenge; `ApiDeps.mcp.oauth` option (issuer, resource) that turns them on; the suite's discovery cases. Until Phase 2 the issuer answers 404, so clients fail at "authorization server metadata", a diagnosable error | ADR-0004 addendum: "No OAuth provider" gains "superseded by ADR-00NN (RFC-0044)" | -| 2 · Authorization server | Better Auth bump; `mcp()` + `jwt()` in `createInstance`; migration `0030_oauth`; `agent_grants`; token → `Principal`; consent screen; `/api/agent-grants`; `cloudhal studio` issuer from `--base-url` (#143); shared suite on sqlite + D1 | ADR-0010 "MCP authorization (Sprint 5)" paragraph (DCR fallback, no DPoP in v1); ADR-0011 §5–6 row "OAuth 2.1 … Sprint 6" → done; new ADR-00NN | +| 2 · Authorization server | Better Auth bump; `mcp()` + `jwt()` in `createInstance`; migration `0031_oauth`; `agent_grants`; token → `Principal`; consent screen; `/api/agent-grants`; `cloudhal studio` issuer from `--base-url` (#143); shared suite on sqlite + D1 | ADR-0010 "MCP authorization (Sprint 5)" paragraph (DCR fallback, no DPoP in v1); ADR-0011 §5–6 row "OAuth 2.1 … Sprint 6" → done; new ADR-00NN | | 3 · Registration and Studio | `cimd()` with the two transports; DCR with limits and pruning; "Connected agents"; `McpClientSetup` tab; `docs/guides/mcp-oauth.md` and the docs site's clients page; hardening guide (secret rotation, allow-list, `CLOUDHAL_MCP_DCR`); e2e test; changeset | `packages/mcp-server/README.md` auth section; `docs/guides/cloudflare-hardening.md` | Each phase is one PR; Phase 1 is independent of the Better Auth bump and can land first. @@ -577,8 +589,10 @@ sequenceDiagram `agent-runtime`, not a contract) gains the optional `grantId`. - **Agent keys:** unchanged, `x-api-key` and `Authorization: Bearer ` keep working; `resolvePrincipal` distinguishes a JWT by shape. The agent-keys suite stays green unchanged. -- **Data:** additive tables (§7). Existing deployments get the challenge on `/mcp` after Phase 1 - with no data change; an installation that never enables the AS loses nothing. +- **Data:** additive tables (§7). Phase 1 ships the challenge behind `McpMountOptions.oauth`, + and no runtime sets that option until Phase 2 gives it an issuer; existing deployments get the + challenge on `/mcp` once a runtime sets it (Phase 2), with no data change. An installation that + never enables the AS loses nothing. - **API:** `/api/agent-grants` is new; `/api/instance` `features` gains `mcpOauth: true|false` so Studio shows the "Connect by URL" tab only where it works. - **CLI:** `cloudhal studio --base-url` (#143) is the dependency; `cloudhal mcp clients create` diff --git a/packages/agent-runtime/src/index.ts b/packages/agent-runtime/src/index.ts index df4115d6..26ee5f1b 100644 --- a/packages/agent-runtime/src/index.ts +++ b/packages/agent-runtime/src/index.ts @@ -10,4 +10,5 @@ export * from "./policy.ts"; export * from "./roles.ts"; export * from "./site-access.ts"; export * from "./skills.ts"; +export * from "./step-up.ts"; export * from "./taxonomy.ts"; diff --git a/packages/agent-runtime/src/policy.ts b/packages/agent-runtime/src/policy.ts index c0feb2bf..c45dea97 100644 --- a/packages/agent-runtime/src/policy.ts +++ b/packages/agent-runtime/src/policy.ts @@ -188,6 +188,11 @@ const OWN_VARIANTS: Partial v.scope) : []; +} + /** * What a call does, decided from its arguments (RFC-0007 §10.2), for tools whose policy depends * on them (`moderate_comments` may hide in `edits` mode but never approve): @@ -307,7 +312,16 @@ export type ApprovalCause = "tool" | "destructive" | "skill" | "manual-mode" | " export type Decision = | { kind: "allow"; limit?: OwnLimit } - | { kind: "deny"; reason: string; code?: "missing_scope" | "mode" | "skill" } + | { + kind: "deny"; + reason: string; + code?: "missing_scope" | "mode" | "skill"; + /** + * `missing_scope` only: the required scopes (`proposeScopes` for a propose call) that + * neither the caller's scopes nor an own variant of them satisfy (RFC-0044 §1 step-up). + */ + missing?: Scope[]; + } | { kind: "approval-required"; reason: string; @@ -379,7 +393,12 @@ export class PolicyEngine { missing.push(s); } if (missing.length > 0) - return { kind: "deny", reason: `missing scope ${missing.join(", ")}`, code: "missing_scope" }; + return { + kind: "deny", + reason: `missing scope ${missing.join(", ")}`, + code: "missing_scope", + missing, + }; if (principal.skill) { // A skill may only call tools inside the scopes it declared (own variants count). const declared = principal.skill.scopes; diff --git a/packages/agent-runtime/src/step-up.ts b/packages/agent-runtime/src/step-up.ts new file mode 100644 index 00000000..4a0735a4 --- /dev/null +++ b/packages/agent-runtime/src/step-up.ts @@ -0,0 +1,57 @@ +/** + * RFC-0044 §1 step-up: which scopes an OAuth connection must add for a call it was denied. The + * answer comes from the same decision the MCP handler makes (`PolicyEngine.evaluate`), so it + * follows the own-variant lattice, a propose call's `proposeScopes`, and the member's role on the + * call's site (RFC-0034 A2), not their installation role. + */ +import { + holds, + isKeyScope, + ownVariantsOf, + type PolicyEngine, + type Principal, + type Scope, +} from "./policy.ts"; +import { roleScopes } from "./roles.ts"; +import { forInstallation, forSite, INSTALLATION_SCOPES, isSiteLimited } from "./site-access.ts"; + +/** + * The scopes to ask for so that `principal` may make the call `toolName(args)`, each the + * strongest one the member's role on the call's site gives a token (`site:publish:own` for an + * Author, not `site:publish`). Undefined when the call is not denied for a missing scope, when + * the principal has no person behind it, or when the role cannot give every missing scope (a + * challenge would then make the client loop). + */ +export function stepUpScopes( + policy: PolicyEngine, + principal: Principal, + toolName: string, + args: Record = {}, +): Scope[] | undefined { + const tool = policy.tool(toolName); + if (!tool || !principal.person) return undefined; + // The handler judges the call as the principal acts on its site (`handleToolCall`). + const siteId = typeof args.siteId === "string" ? args.siteId : undefined; + const sited = siteId ? forSite(principal, siteId) : forInstallation(principal); + const person = sited?.person; + if (!sited || !person) return undefined; + const decision = policy.evaluate(sited, toolName, { args }); + if (decision.kind !== "deny" || decision.code !== "missing_scope" || !decision.missing?.length) + return undefined; + const role = roleScopes(person.role); + const grantable = + !siteId && isSiteLimited(person) ? role.filter((s) => !INSTALLATION_SCOPES.includes(s)) : role; + const asked: Scope[] = []; + for (const scope of decision.missing) { + const pick = [scope, ...ownVariantsOf(tool, scope)].find( + (s) => isKeyScope(s) && holds(grantable, s), + ); + if (!pick) return undefined; + if (!asked.includes(pick)) asked.push(pick); + } + // With them the scope check must pass, or the client would re-authorize for nothing. + const after = policy.evaluate({ ...sited, scopes: [...sited.scopes, ...asked] }, toolName, { + args, + }); + return after.kind === "deny" && after.code === "missing_scope" ? undefined : asked; +} diff --git a/packages/agent-runtime/test/policy.test.ts b/packages/agent-runtime/test/policy.test.ts index ed1da778..be9f04a1 100644 --- a/packages/agent-runtime/test/policy.test.ts +++ b/packages/agent-runtime/test/policy.test.ts @@ -217,7 +217,12 @@ describe("PolicyEngine", () => { expect(implies("content:write", "content:propose")).toBe(true); expect( body.evaluate({ actor: agent, scopes: ["content:read"] }, "edit_body", propose), - ).toEqual({ kind: "deny", reason: "missing scope content:propose", code: "missing_scope" }); + ).toEqual({ + kind: "deny", + reason: "missing scope content:propose", + code: "missing_scope", + missing: ["content:propose"], + }); // A skill must declare what the call needs. const skilled = { ...planKey, diff --git a/packages/agent-runtime/test/step-up.test.ts b/packages/agent-runtime/test/step-up.test.ts new file mode 100644 index 00000000..759dcff9 --- /dev/null +++ b/packages/agent-runtime/test/step-up.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from "vitest"; +import { type PersonRef, PolicyEngine, type Principal, type Scope } from "../src/policy.ts"; +import type { Role } from "../src/roles.ts"; +import { stepUpScopes } from "../src/step-up.ts"; + +const engine = new PolicyEngine([ + { + name: "update_entry", + scopes: ["content:write"], + annotations: { readOnlyHint: false }, + own: "entry", + }, + { + name: "update_page", + scopes: ["content:write"], + annotations: { readOnlyHint: false }, + }, + { + name: "publish", + scopes: ["site:publish"], + annotations: { destructiveHint: true }, + requiresApproval: true, + own: "changeset", + }, + { + name: "edit_body", + scopes: ["content:write"], + annotations: { readOnlyHint: false }, + own: "entry", + classify: (a) => (a.mode === "propose" ? "propose" : undefined), + proposeScopes: ["content:read", "content:propose"], + }, + { name: "audit_log", scopes: ["audit:read"], annotations: { readOnlyHint: true } }, +]); + +/** A token principal of RFC-0044 phase 2: an agent with a grant, its scopes the token's. */ +const grant = (role: Role, scopes: Scope[], sites?: PersonRef["sites"]): Principal => ({ + actor: { kind: "agent", id: "agent_grant0001" }, + scopes, + keyScopes: scopes, + mode: "edits", + grantId: "grant_0001", + person: { id: "user_aaaaaaaa", userId: "u1", role, ...(sites ? { sites } : {}) }, +}); +const READ: Scope[] = ["site:read", "content:read"]; + +describe("stepUpScopes (RFC-0044 §1)", () => { + it("reports what the denial lacks, from the policy engine", () => { + expect(engine.evaluate(grant("editor", READ), "update_entry")).toMatchObject({ + kind: "deny", + code: "missing_scope", + missing: ["content:write"], + }); + expect(stepUpScopes(engine, grant("editor", READ), "update_entry")).toEqual(["content:write"]); + expect(stepUpScopes(engine, grant("editor", READ), "publish")).toEqual(["site:publish"]); + }); + + it("asks an Author for the own variant their role holds", () => { + expect(stepUpScopes(engine, grant("author", READ), "update_entry")).toEqual([ + "content:write:own", + ]); + expect(stepUpScopes(engine, grant("author", READ), "publish")).toEqual(["site:publish:own"]); + // A tool without `own` is not satisfied by an own variant: no challenge, no loop. + expect(stepUpScopes(engine, grant("author", READ), "update_page")).toBeUndefined(); + }); + + it("asks a Contributor for the draft variant, and nothing their role lacks", () => { + expect(stepUpScopes(engine, grant("contributor", READ), "update_entry")).toEqual([ + "content:draft:own", + ]); + expect(stepUpScopes(engine, grant("contributor", READ), "publish")).toBeUndefined(); + }); + + it("asks a propose call for proposeScopes, not the write it proposes", () => { + const propose = { mode: "propose" }; + expect(stepUpScopes(engine, grant("editor", ["site:read"]), "edit_body", propose)).toEqual([ + "content:read", + "content:propose", + ]); + // The same tool applying the change needs the write. + expect(stepUpScopes(engine, grant("editor", READ), "edit_body", {})).toEqual(["content:write"]); + }); + + it("judges the member's role on the call's site, not the installation role", () => { + const limited = grant("editor", READ, { site_a: "contributor", site_b: "editor" }); + expect(stepUpScopes(engine, limited, "update_entry", { siteId: "site_a" })).toEqual([ + "content:draft:own", + ]); + expect(stepUpScopes(engine, limited, "update_entry", { siteId: "site_b" })).toEqual([ + "content:write", + ]); + // The installation role (Editor) holds site:publish; the Contributor on site_a does not. + expect(stepUpScopes(engine, limited, "publish", { siteId: "site_a" })).toBeUndefined(); + // A site the member cannot open: the handler answers not_found, never a challenge. + expect(stepUpScopes(engine, limited, "update_entry", { siteId: "site_c" })).toBeUndefined(); + }); + + it("never asks for a person-only scope, and never for a call that is not a scope denial", () => { + expect(stepUpScopes(engine, grant("administrator", READ), "audit_log")).toBeUndefined(); + const writer = grant("editor", [...READ, "content:write"]); + expect(stepUpScopes(engine, writer, "update_entry")).toBeUndefined(); + expect(stepUpScopes(engine, { ...writer, mode: "plan" }, "update_entry")).toBeUndefined(); + expect(stepUpScopes(engine, grant("editor", READ), "no_such_tool")).toBeUndefined(); + const { person: _, ...anonymous } = grant("editor", READ); + expect(stepUpScopes(engine, anonymous, "update_entry")).toBeUndefined(); + }); +}); diff --git a/packages/analytics/src/wae/index.ts b/packages/analytics/src/wae/index.ts index 68239f54..ddfa9d01 100644 --- a/packages/analytics/src/wae/index.ts +++ b/packages/analytics/src/wae/index.ts @@ -24,9 +24,13 @@ export { WaeWriter } from "./writer.ts"; /** The dataset name for a deployment: `_events`, `-` → `_`, lower case (RFC-0006 §4). */ export function datasetName(deployment: string): string { - const base = deployment - .toLowerCase() - .replace(/[^a-z0-9_]+/g, "_") - .replace(/^_+|_+$/g, ""); + const underscored = deployment.toLowerCase().replace(/[^a-z0-9_]+/g, "_"); + // Linear trim: `/^_+|_+$/g` backtracks on a long run of `_` inside the name (CodeQL + // js/polynomial-redos), and the replace above keeps runs the name already had. + let start = 0; + let end = underscored.length; + while (start < end && underscored[start] === "_") start++; + while (end > start && underscored[end - 1] === "_") end--; + const base = underscored.slice(start, end); return `${base || "cloudhal"}_events`; } diff --git a/packages/analytics/test/wae-reader.test.ts b/packages/analytics/test/wae-reader.test.ts index e6f3a32e..7f936ec0 100644 --- a/packages/analytics/test/wae-reader.test.ts +++ b/packages/analytics/test/wae-reader.test.ts @@ -167,5 +167,16 @@ describe("WaeReader", () => { it("names datasets after the deployment", () => { expect(datasetName("newsroom")).toBe("newsroom_events"); expect(datasetName("My-Site 2")).toBe("my_site_2_events"); + expect(datasetName("__news__room__")).toBe("news__room_events"); + expect(datasetName("___")).toBe("cloudhal_events"); + }); + + it("trims a dataset name in linear time (#168, CodeQL js/polynomial-redos)", () => { + // `/^_+|_+$/g` took about 4 s on a 100k run of `_` inside the name (the replace before it + // keeps such runs); the loop takes well under a millisecond. + const name = `a${"_".repeat(200_000)}b`; + const start = performance.now(); + expect(datasetName(name)).toBe(`${name}_events`); + expect(performance.now() - start).toBeLessThan(1500); }); }); diff --git a/packages/api/src/auth/principal.ts b/packages/api/src/auth/principal.ts index 7d3e9b2a..1ea7b4c6 100644 --- a/packages/api/src/auth/principal.ts +++ b/packages/api/src/auth/principal.ts @@ -31,7 +31,7 @@ export function createPrincipalResolver(d: AuthDeps): PrincipalResolver { if (key) { const verified = await api.verifyApiKey({ body: { key } }); if (!verified.valid || !verified.key) return undefined; - // A fixed window per key (`rateLimit` table): it resets every window however busy the key is. + // A fixed window per key (`rate_limits` table): it resets every window however busy the key is. if ( !(await staffStore.hit(`cloudhal:key:${verified.key.id}`, keyLimit.windowMs, keyLimit.max)) ) diff --git a/packages/api/src/mcp-mount.ts b/packages/api/src/mcp-mount.ts index c7cd7fcc..5ca2600d 100644 --- a/packages/api/src/mcp-mount.ts +++ b/packages/api/src/mcp-mount.ts @@ -10,6 +10,8 @@ import type { BuildHub } from "./build.ts"; import type { ApiDeps, Env } from "./context.ts"; import { unauthorized } from "./errors.ts"; import { + assertMcpOauth, + MCP_PATH, mountProtectedResourceMetadata, stepUpChallenge, unauthorizedChallenge, @@ -38,8 +40,11 @@ export function mountMcp( schedules: parts.schedules, }); const oauth = deps.mcp.oauth; - if (oauth) mountProtectedResourceMetadata(app, oauth); - app.all("/mcp", async (c) => { + if (oauth) { + assertMcpOauth(oauth); + mountProtectedResourceMetadata(app, oauth); + } + app.all(MCP_PATH, async (c) => { // DNS-rebinding / cross-site protection: browsers send Origin; it must be this origin. const origin = c.req.header("origin"); if (origin && origin !== new URL(c.req.url).origin) @@ -59,7 +64,14 @@ export function mountMcp( 403, ); if (oauth) { - const stepUp = await stepUpChallenge(c.req.raw, principal, c.get("policy")(), oauth); + const stepUp = await stepUpChallenge({ + request: c.req.raw, + principal, + policy: c.get("policy")(), + oauth, + audit: deps.mcp.audit, + now: deps.now, + }); if (stepUp) return stepUp; } return mcp.fetch(c.req.raw, principal); diff --git a/packages/api/src/mcp-oauth.ts b/packages/api/src/mcp-oauth.ts index ea826282..1acd4430 100644 --- a/packages/api/src/mcp-oauth.ts +++ b/packages/api/src/mcp-oauth.ts @@ -10,13 +10,13 @@ * exist, so clients fail at "authorization server metadata", a diagnosable error (RFC-0044 §9). */ import { - holds, - isKeyScope, + type AuditLog, + effectiveMode, type PolicyEngine, type Principal, - roleScopes, type Scope, scopesForMode, + stepUpScopes, } from "@cloudhal/agent-runtime"; import type { Context, Hono } from "hono"; import type { Env } from "./context.ts"; @@ -53,6 +53,22 @@ export function canonicalResource(resource: string): string { return url.href.replace(/\/$/, ""); } +/** The path `createApi` mounts the MCP endpoint at (ADR-0011). */ +export const MCP_PATH = "/mcp"; + +/** + * Fails `createApi` when `resource` cannot name this endpoint: tokens are bound to it (RFC 8707), + * so a resource that does not end in `/mcp` would make every token's audience wrong. A proxy may + * put a prefix in front (`https://example.com/studio/mcp`). + */ +export function assertMcpOauth(oauth: McpOauthOptions): void { + const resource = new URL(canonicalResource(oauth.resource)); + if (!resource.pathname.endsWith(MCP_PATH) || resource.search) + throw new Error( + `mcp.oauth.resource must be the public URL of ${MCP_PATH} (e.g. https://studio.example.com${MCP_PATH}), got ${oauth.resource}`, + ); +} + /** The URL of the path-aware metadata document, named in every challenge. */ export function resourceMetadataUrl(oauth: McpOauthOptions): string { const url = new URL(canonicalResource(oauth.resource)); @@ -156,30 +172,42 @@ async function toolCallOf( return { name, args: args && typeof args === "object" ? (args as Record) : {} }; } +export interface StepUpOptions { + readonly request: Request; + readonly principal: Principal; + readonly policy: PolicyEngine; + readonly oauth: McpOauthOptions; + /** Where the step-up 403 is recorded, like every other denial (`runTool`). */ + readonly audit?: AuditLog | undefined; + readonly now?: (() => Date) | undefined; +} + /** * RFC-0044 §1 step-up: a token principal (`grantId`) calling a tool whose scopes the token lacks - * but the member's role holds gets `403 insufficient_scope` naming every missing scope at once, - * so the client re-authorizes with the union (MCP "Scope Challenge Handling"). Every other denial - * (role, mode, `:own`, approvals) stays a JSON-RPC result from the handler, and a key principal - * is never challenged: a key cannot re-authorize. + * but the member's role on the call's site gives gets `403 insufficient_scope` naming every + * missing scope at once, so the client re-authorizes with the union (MCP "Scope Challenge + * Handling"). The set is the policy engine's (`stepUpScopes`): own variants, a propose call's + * scopes, the site role. Every other denial (role, mode, `:own`, approvals) stays a JSON-RPC + * result from the handler, and a key principal is never challenged: a key cannot re-authorize. */ -export async function stepUpChallenge( - request: Request, - principal: Principal, - policy: PolicyEngine, - oauth: McpOauthOptions, -): Promise { +export async function stepUpChallenge(options: StepUpOptions): Promise { + const { request, principal, policy, oauth } = options; if (!principal.grantId || !principal.person) return undefined; const call = await toolCallOf(request); if (!call) return undefined; - const tool = policy.tool(call.name); - if (!tool) return undefined; - const decision = policy.evaluate(principal, call.name, { args: call.args }); - if (decision.kind !== "deny" || decision.code !== "missing_scope") return undefined; - const missing = tool.scopes.filter((s) => !holds(principal.scopes, s)); - const role = roleScopes(principal.person.role); - // Only scopes a token can carry and the role holds: anything else would make the client loop. - if (!missing.length || !missing.every((s) => isKeyScope(s) && holds(role, s))) return undefined; + const missing = stepUpScopes(policy, principal, call.name, call.args); + if (!missing) return undefined; + const reason = `insufficient_scope: this connection needs ${missing.join(", ")}`; + await options.audit?.record({ + at: (options.now?.() ?? new Date()).toISOString(), + actor: principal.actor, + siteId: typeof call.args.siteId === "string" ? call.args.siteId : undefined, + tool: call.name, + decision: "deny", + reason, + mode: effectiveMode(principal), + outcome: "error", + }); return Response.json( { error: "insufficient_scope", diff --git a/packages/api/src/playground.ts b/packages/api/src/playground.ts index 17e033f1..2997a574 100644 --- a/packages/api/src/playground.ts +++ b/packages/api/src/playground.ts @@ -211,14 +211,19 @@ export function playgroundDenyRule(method: string, pattern: string): PlaygroundB return undefined; } +/** The API's well-known documents (RFC-0044 §1): judged like `/api/*`, so their rows apply. */ +const WELL_KNOWN_API = "/.well-known/oauth-protected-resource"; + /** * Why `method path` is refused in playground mode, or undefined when it is allowed. Only API - * paths (`/api/*`, `/mcp`) are judged; anything else (Studio, the rendered site) is not the API's. + * paths (`/api/*`, `/mcp`, the API's well-known documents) are judged; anything else (Studio, + * the rendered site) is not the API's. */ export function isBlockedInPlayground(method: string, path: string): PlaygroundBlock | undefined { const clean = path.replace(/\/+$/, "") || "/"; if (clean === "/mcp" || clean.startsWith("/mcp/")) return "mcp"; - if (!clean.startsWith("/api/")) return undefined; + const wellKnown = clean === WELL_KNOWN_API || clean.startsWith(`${WELL_KNOWN_API}/`); + if (!clean.startsWith("/api/") && !wellKnown) return undefined; const m = method.toUpperCase() === "HEAD" ? "GET" : method.toUpperCase(); // Prefix rules first, so an undeclared path under a blocked area names its area. const route = routeFor(m, clean); diff --git a/packages/api/src/staff.ts b/packages/api/src/staff.ts index 6cdf50d0..71534e89 100644 --- a/packages/api/src/staff.ts +++ b/packages/api/src/staff.ts @@ -534,18 +534,20 @@ export class StaffStore { // ───────────────────────────── rate limits ───────────────────────────── /** - * Fixed-window counter in Better Auth's `rateLimit` table (migration 0005), under keys of our own - * (`cloudhal:…`). Returns false once `max` requests were seen within `windowMs`. + * Fixed-window counter under keys of our own (`cloudhal:…`), in Cloudhal's `rate_limits` table + * (migration 0030), not Better Auth's `rateLimit`: Better Auth prunes that by its own 60 s + * window, which reset hourly counters every minute (#168). One upsert, so concurrent hits never + * lose a count. Returns false once `max` requests were seen within `windowMs`. */ async hit(key: string, windowMs: number, max: number): Promise { const now = this.#now().getTime(); const start = now - windowMs; await this.#sql.run( - `INSERT INTO "rateLimit" ("id", "key", "count", "lastRequest") VALUES (?, ?, 1, ?) ON CONFLICT ("key") DO UPDATE SET "count" = CASE WHEN "rateLimit"."lastRequest" < ? THEN 1 ELSE "rateLimit"."count" + 1 END, "lastRequest" = CASE WHEN "rateLimit"."lastRequest" < ? THEN excluded."lastRequest" ELSE "rateLimit"."lastRequest" END`, - [newRowId(), key, now, start, start], + "INSERT INTO rate_limits (key, count, window_start) VALUES (?, 1, ?) ON CONFLICT (key) DO UPDATE SET count = CASE WHEN rate_limits.window_start < ? THEN 1 ELSE rate_limits.count + 1 END, window_start = CASE WHEN rate_limits.window_start < ? THEN excluded.window_start ELSE rate_limits.window_start END", + [key, now, start, start], ); const row = await this.#sql.get<{ count: number }>( - `SELECT "count" FROM "rateLimit" WHERE "key" = ?`, + "SELECT count FROM rate_limits WHERE key = ?", [key], ); return Number(row?.count ?? 0) <= max; diff --git a/packages/api/src/testing-mcp-oauth.ts b/packages/api/src/testing-mcp-oauth.ts index 1eccd049..23b972ef 100644 --- a/packages/api/src/testing-mcp-oauth.ts +++ b/packages/api/src/testing-mcp-oauth.ts @@ -5,7 +5,7 @@ * better-sqlite3 and Miniflare D1 (the step-up's key-principal case reads the key from the * database). */ -import { type Principal, scopesForMode } from "@cloudhal/agent-runtime"; +import { AuditLog, type PersonRef, type Principal, scopesForMode } from "@cloudhal/agent-runtime"; import { seedStarterSite } from "@cloudhal/core-ts"; import { afterAll, describe, expect, it } from "vitest"; import { createApi } from "./app.ts"; @@ -41,6 +41,8 @@ function toolCall(name: string, args: Json): RequestInit { }; } +const entryArgs = (siteId: string): Json => ({ siteId, slug: "x", title: "X", body: "x" }); + export function describeMcpOauthDiscovery(label: string, backend: RolesBackend): void { describe(`MCP OAuth discovery: ${label}`, { timeout: 180_000 }, () => { afterAll(async () => { @@ -84,29 +86,52 @@ export function describeMcpOauthDiscovery(label: string, backend: RolesBackend): }; /** A deployment whose principal is injected: a token principal of Phase 2, before Phase 2. */ - const setupWithPrincipal = async (principal: Principal | undefined) => { + const setupWithPrincipal = async ( + principal: Principal | undefined | ((siteId: string) => Principal), + ) => { const b = await backend.create(); const site = await seedStarterSite(b.store, { name: "Blog", slug: "blog" }); + const audit = new AuditLog(b.sql); + const resolved = typeof principal === "function" ? principal(site.id) : principal; const deps: ApiDeps = { store: b.store, - resolvePrincipal: async () => principal, - mcp: { stateKey: "mcp-oauth-state-key-0123456789abcdefghij", oauth: OAUTH }, + resolvePrincipal: async () => resolved, + mcp: { stateKey: "mcp-oauth-state-key-0123456789abcdefghij", oauth: OAUTH, audit }, }; - return { app: createApi(deps), siteId: site.id }; + return { app: createApi(deps), siteId: site.id, audit }; }; const grantPrincipal = ( - role: "editor" | "contributor", + role: PersonRef["role"], scopes: Principal["scopes"], + sites?: PersonRef["sites"], ): Principal => ({ actor: { kind: "agent", id: "agent_grant0001", name: "Claude" }, scopes, mode: "edits", grantId: "grant_0001", keyScopes: scopes, - person: { id: "user_owner1", userId: "owner1", role, name: "Olive Owner" }, + person: { + id: "user_owner1", + userId: "owner1", + role, + name: "Olive Owner", + ...(sites ? { sites } : {}), + }, }); + /** The scopes a step-up 403 names, or undefined when the call was not challenged. */ + const challenged = async ( + principal: Principal | ((siteId: string) => Principal), + tool: string, + args: (siteId: string) => Json, + ) => { + const { app, siteId } = await setupWithPrincipal(principal); + const res = await app.request(`${ORIGIN}/mcp`, toolCall(tool, args(siteId))); + if (res.status !== 403) return undefined; + return ((await res.json()) as { details: { scope: string[] } }).details.scope; + }; + it("answers 401 with the RFC 9728 challenge and the scopes to request", async () => { const { app } = await setup(OAUTH); const res = await app.request(`${ORIGIN}/mcp`, toolCall("get_mode", {})); @@ -187,6 +212,85 @@ export function describeMcpOauthDiscovery(label: string, backend: RolesBackend): expect(reached.headers.get("www-authenticate")).toBeNull(); }); + it("asks for the own variant an Author's or Contributor's role holds", async () => { + const read: Principal["scopes"] = ["site:read", "content:read"]; + expect(await challenged(grantPrincipal("author", read), "create_entry", entryArgs)).toEqual([ + "content:write:own", + ]); + expect( + await challenged(grantPrincipal("contributor", read), "create_entry", entryArgs), + ).toEqual(["content:draft:own"]); + expect( + await challenged(grantPrincipal("author", read), "publish", (siteId) => ({ siteId })), + ).toEqual(["site:publish:own"]); + }); + + it("asks a propose call for the propose scopes, not the write", async () => { + const propose = (siteId: string) => ({ siteId, entryId: "e1", ops: [], mode: "propose" }); + expect( + await challenged(grantPrincipal("editor", ["site:read"]), "edit_body", propose), + ).toEqual(["content:read", "content:propose"]); + }); + + it("judges the member's role on the call's site, not the installation role", async () => { + // An Editor of the installation who is a Contributor on this site: no publish to ask for, + // and the draft variant (not content:write) for a new entry. + const read: Principal["scopes"] = ["site:read", "content:read"]; + const limited = (siteId: string) => + grantPrincipal("editor", read, { [siteId]: "contributor" }); + const { app, siteId } = await setupWithPrincipal(limited); + const res = await app.request(`${ORIGIN}/mcp`, toolCall("publish", { siteId })); + expect(res.status).not.toBe(403); + expect(res.headers.get("www-authenticate")).toBeNull(); + const entry = await app.request(`${ORIGIN}/mcp`, toolCall("create_entry", entryArgs(siteId))); + expect(entry.status).toBe(403); + expect(await entry.json()).toMatchObject({ details: { scope: ["content:draft:own"] } }); + // Where the same member is an Editor, the challenge names content:write. + const editorOn = (siteId: string) => + grantPrincipal("contributor", read, { [siteId]: "editor" }); + expect(await challenged(editorOn, "create_entry", entryArgs)).toEqual(["content:write"]); + // A site the member cannot open answers not_found from the handler, never a challenge. + const elsewhere = grantPrincipal("editor", read, { site_other: "editor" }); + expect(await challenged(elsewhere, "create_entry", entryArgs)).toBeUndefined(); + }); + + it("records the step-up 403 in the audit log, like any other denial", async () => { + const { app, siteId, audit } = await setupWithPrincipal( + grantPrincipal("editor", ["site:read", "content:read"]), + ); + const res = await app.request(`${ORIGIN}/mcp`, toolCall("publish", { siteId })); + expect(res.status).toBe(403); + const rows = await audit.list({ tool: "publish" }); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + actor: { kind: "agent", id: "agent_grant0001" }, + siteId, + tool: "publish", + decision: "deny", + outcome: "error", + reason: "insufficient_scope: this connection needs site:publish", + }); + }); + + it("refuses at createApi time an oauth.resource that is not the /mcp endpoint", async () => { + const b = await backend.create(); + const make = (resource: string) => + createApi({ + store: b.store, + mcp: { + stateKey: "mcp-oauth-state-key-0123456789abcdefghij", + oauth: { issuer: `${ORIGIN}/api/auth`, resource }, + }, + }); + expect(() => make(`${ORIGIN}/`)).toThrow( + /mcp\.oauth\.resource must be the public URL of \/mcp/, + ); + expect(() => make(`${ORIGIN}/api/mcp-tools`)).toThrow(/mcp\.oauth\.resource/); + expect(() => make(`${ORIGIN}/mcp?x=1`)).toThrow(/mcp\.oauth\.resource/); + expect(() => make(`${ORIGIN}/mcp/`)).not.toThrow(); + expect(() => make("https://example.com/studio/mcp")).not.toThrow(); + }); + it("never challenges for a scope the role lacks, and never challenges a key", async () => { // A Contributor's role has no site:publish: a challenge would make the client loop. const contributor = await setupWithPrincipal( diff --git a/packages/api/src/testing-rate-limits.ts b/packages/api/src/testing-rate-limits.ts new file mode 100644 index 00000000..1f224482 --- /dev/null +++ b/packages/api/src/testing-rate-limits.ts @@ -0,0 +1,98 @@ +/** + * Shared suite for #168 item 8: Cloudhal's own long-window counters survive Better Auth's + * rate-limit pruning. Better Auth deletes every `rateLimit` row older than its longest window + * (60 s) whenever one of its own windows lapses; the hourly reset-mail cap used to live there and + * restarted every minute. Runs the real API and Better Auth (with database rate limits, as + * production does) on any backend; `Date` is faked so Better Auth's clock moves too. + */ +import { seedStarterSite } from "@cloudhal/core-ts"; +import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; +import { createApi } from "./app.ts"; +import { createAuth } from "./auth.ts"; +import type { MailMessage } from "./mail.ts"; +import type { RolesBackend } from "./testing-roles.ts"; + +const ORIGIN = "http://limits.test"; +const IP = "203.0.113.7"; +const T0 = Date.parse("2026-11-08T12:00:00.000Z"); + +export function describeRateLimitCounters(label: string, backend: RolesBackend): void { + describe(`Cloudhal rate-limit counters: ${label}`, { timeout: 180_000 }, () => { + afterEach(() => { + vi.useRealTimers(); + }); + afterAll(async () => { + await backend.dispose?.(); + }); + + it("keeps the hourly reset-mail cap when Better Auth prunes its 60 s window", async () => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(T0); + const b = await backend.create(); + await seedStarterSite(b.store, { name: "Limits", slug: "limits" }); + const auth = createAuth({ + database: b.database, + sql: b.sql, + baseURL: ORIGIN, + secret: "rate-limit-test-secret-0123456789abcdefg", + signUp: "first-user", + clientAddressHeader: "cf-connecting-ip", + }); + const mailbox: MailMessage[] = []; + const pending: Promise[] = []; + const app = createApi({ + store: b.store, + resolvePrincipal: auth.resolvePrincipal, + authHandler: auth.handler, + staff: auth.staff, + signUp: auth.signUp, + mail: { id: "test", send: async (m) => void mailbox.push(m) }, + defer: (task) => void pending.push(task), + }); + const post = (path: string, body: unknown) => + app.request(`${ORIGIN}${path}`, { + method: "POST", + headers: { "content-type": "application/json", "cf-connecting-ip": IP }, + body: JSON.stringify(body), + }); + const email = "owner@example.com"; + const signedUp = await post("/api/auth/sign-up/email", { + name: "Olive Owner", + email, + password: "correct horse battery staple", + }); + expect(signedUp.status).toBe(200); + const signIn = () => + post("/api/auth/sign-in/email", { email, password: "not the password at all" }); + expect((await signIn()).status).toBe(401); + const askForResets = async (n: number) => { + for (let i = 0; i < n; i++) + expect((await post("/api/password-reset/request", { email })).status).toBe(202); + while (pending.length > 0) await Promise.all(pending.splice(0)); + return mailbox.filter((m) => m.to === email).length; + }; + + // The first burst reaches the cap of 3 mails per address per hour. + expect(await askForResets(4)).toBe(3); + + // 61 s later Better Auth's own sign-in window for this address has lapsed: its next sign-in + // prunes every `rateLimit` row older than 60 s (the sentinel proves the prune ran). + await b.sql.run( + `INSERT INTO "rateLimit" ("id", "key", "count", "lastRequest") VALUES ('sentinel', 'sentinel', 1, ?)`, + [T0], + ); + vi.setSystemTime(T0 + 61_000); + expect((await signIn()).status).toBe(401); + expect(await b.sql.get(`SELECT "key" FROM "rateLimit" WHERE "key" = 'sentinel'`)).toBe( + undefined, + ); + + // The second burst, inside the same hour, still sees the cap. + expect(await askForResets(3)).toBe(3); + + // An hour after the first mail the address can be mailed again. + vi.setSystemTime(T0 + 61 * 60_000); + expect(await askForResets(1)).toBe(4); + }); + }); +} diff --git a/packages/api/src/tokens.ts b/packages/api/src/tokens.ts index 5c114065..0f325ce7 100644 --- a/packages/api/src/tokens.ts +++ b/packages/api/src/tokens.ts @@ -70,8 +70,8 @@ export function clientIp(c: Context): string { } /** - * 10 requests per minute per IP on a public token endpoint (RFC-0005 §7.1), counted in Better - * Auth's `rateLimit` table. + * 10 requests per minute per IP on a public token endpoint (RFC-0005 §7.1), counted in Cloudhal's + * `rate_limits` table (migration 0030; not Better Auth's `rateLimit`, which it prunes by 60 s). */ export async function limitPublic(c: Context, bucket: string): Promise { const staff = c.get("staff"); diff --git a/packages/api/test/playground.test.ts b/packages/api/test/playground.test.ts index d94d361d..a22f3122 100644 --- a/packages/api/test/playground.test.ts +++ b/packages/api/test/playground.test.ts @@ -79,6 +79,10 @@ describe("the playground route table", () => { ["POST", "/api/agent-keys", "keys"], ["POST", "/mcp", "mcp"], ["GET", "/mcp/", "mcp"], + // RFC-0044 decision 10: no OAuth discovery in a playground, even if `mcp.oauth` were set. + ["GET", "/.well-known/oauth-protected-resource", "mcp"], + ["HEAD", "/.well-known/oauth-protected-resource/mcp", "mcp"], + ["GET", "/.well-known/oauth-protected-resource/other", "unclassified"], ["POST", "/api/sites/s1/assets/uploads", "uploads"], ["PUT", "/api/sites/s1/assets/uploads/u1", "uploads"], ["DELETE", "/api/sites/s1/assets/a1", "uploads"], @@ -113,6 +117,7 @@ describe("the playground route table", () => { // not the API's to judge: Studio and the rendered site ["GET", "/studio/members"], ["GET", "/s/01J0000000/blog/"], + ["GET", "/.well-known/security.txt"], ])("allows %s %s", (method, path) => { expect(isBlockedInPlayground(method, path)).toBeUndefined(); }); @@ -162,6 +167,28 @@ describe("createApi({ playground })", () => { expect(instance.features.mcp).toBe(false); }); + it("refuses the OAuth discovery documents even when mcp.oauth is configured", async () => { + const sql = await BetterSqlite3Driver.open(":memory:"); + const store = new SiteGraphStore({ sql, blobs: new MemoryBlobStore() }); + await store.init(); + const app = createApi({ + store, + playground: { info: () => info }, + mcp: { + stateKey: "playground-state-key-0123456789abcdefghij", + oauth: { issuer: "http://try.test/api/auth", resource: "http://try.test/mcp" }, + }, + }); + for (const path of [ + "/.well-known/oauth-protected-resource", + "/.well-known/oauth-protected-resource/mcp", + ]) { + const res = await app.request(`http://try.test${path}`); + expect(res.status, path).toBe(403); + expect(await res.json()).toMatchObject({ error: "playground", block: "mcp" }); + } + }); + it("edits, snapshots and publishes", async () => { const { app, site, store } = await setup(); expect((await app.request(`/api/sites/${site.id}`)).status).toBe(200); diff --git a/packages/api/test/rate-limits.test.ts b/packages/api/test/rate-limits.test.ts new file mode 100644 index 00000000..80cecbf9 --- /dev/null +++ b/packages/api/test/rate-limits.test.ts @@ -0,0 +1,34 @@ +import { SiteGraphStore } from "@cloudhal/core-ts"; +import { MemoryBlobStore } from "@cloudhal/storage"; +import { D1Driver } from "@cloudhal/storage/d1"; +import { BetterSqlite3Driver } from "@cloudhal/storage/sqlite"; +import { createMiniflareD1, createTableReset } from "@cloudhal/storage/testing"; +import Database from "better-sqlite3"; +import { describeRateLimitCounters } from "../src/testing-rate-limits.ts"; + +describeRateLimitCounters("better-sqlite3", { + async create() { + const database = new Database(":memory:"); + const sql = new BetterSqlite3Driver(database); + const blobs = new MemoryBlobStore(); + const store = new SiteGraphStore({ sql, blobs }); + await store.init(); + return { store, sql, blobs, database }; + }, +}); + +let d1: Awaited> | undefined; +const reset = createTableReset(); +describeRateLimitCounters("Cloudflare D1 (Miniflare)", { + async create() { + d1 ??= await createMiniflareD1(); + const sql = new D1Driver(d1.db); + const blobs = new MemoryBlobStore(); + const store = new SiteGraphStore({ sql, blobs }); + await reset(sql, () => store.init()); + return { store, sql, blobs, database: d1.db }; + }, + async dispose() { + await d1?.dispose(); + }, +}); diff --git a/packages/astro-renderer/src/markdown.ts b/packages/astro-renderer/src/markdown.ts index a6dea292..4fdb2f38 100644 --- a/packages/astro-renderer/src/markdown.ts +++ b/packages/astro-renderer/src/markdown.ts @@ -3,6 +3,7 @@ import { type BlockRegistry, createBlockRegistry } from "@cloudhal/core-ts/block import { type DirectiveToken, directiveExtension } from "@cloudhal/core-ts/body-blocks"; import { checkDirective, directiveProps } from "@cloudhal/core-ts/directives"; import { ASSET_HREF, assetUrl, mediaPath } from "@cloudhal/core-ts/media"; +import { trimChar } from "@cloudhal/core-ts/names"; import { parseSurfaceFence } from "@cloudhal/core-ts/surfaces"; import { Marked } from "marked"; import { builtInBlockHtml, isBuiltInBlock, missingBlockHtml } from "./block-html.ts"; @@ -48,17 +49,14 @@ const RESERVED_ID = /^(?:comment|cloudhal|ch-)/; * Latin letters or digits falls back to `section`; repeats get `-2`, `-3`, …. */ export function headingSlug(text: string, taken?: Map): string { - const base = - text - .replace(/[ıİşŞğĞ]/g, (c) => TURKISH[c] ?? c) - .normalize("NFKD") - .replace(/[̀-ͯ]/g, "") - .toLowerCase() - .replace(/ß/g, "ss") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 64) - .replace(/-+$/, "") || "section"; + const dashed = text + .replace(/[ıİşŞğĞ]/g, (c) => TURKISH[c] ?? c) + .normalize("NFKD") + .replace(/[̀-ͯ]/g, "") + .toLowerCase() + .replace(/ß/g, "ss") + .replace(/[^a-z0-9]+/g, "-"); + const base = trimChar(trimChar(dashed, "-").slice(0, 64), "-") || "section"; const slug = RESERVED_ID.test(base) || /^\d/.test(base) ? `h-${base}` : base; if (!taken) return slug; const seen = taken.get(slug) ?? 0; diff --git a/packages/astro-renderer/test/kit.test.ts b/packages/astro-renderer/test/kit.test.ts index e6391282..40996351 100644 --- a/packages/astro-renderer/test/kit.test.ts +++ b/packages/astro-renderer/test/kit.test.ts @@ -24,6 +24,12 @@ describe("heading ids (RFC-0018 §3)", () => { expect(headingSlug("Café façade — Straße")).toBe("cafe-facade-strasse"); }); + it("trims dashes at both ends and at the 64-character cut", () => { + expect(headingSlug(" — Notes — ")).toBe("notes"); + expect(headingSlug(`${"a".repeat(63)} b`)).toBe("a".repeat(63)); + expect(headingSlug("-".repeat(100_000))).toBe("section"); + }); + it("never takes an id Cloudhal's own markup uses, nor starts with a digit", () => { expect(headingSlug("Comments")).toBe("h-comments"); expect(headingSlug("Cloudhal setup")).toBe("h-cloudhal-setup"); diff --git a/packages/cli/test/studio-selfhost.test.ts b/packages/cli/test/studio-selfhost.test.ts index 823d6665..45b05b4f 100644 --- a/packages/cli/test/studio-selfhost.test.ts +++ b/packages/cli/test/studio-selfhost.test.ts @@ -257,14 +257,21 @@ describe("studio auth options (#165)", () => { expect(() => assertProxyHasBaseUrl({ ...server, hostname: "0.0.0.0" })).not.toThrow(); }); - it("makes slugs and setup lines in linear time (CodeQL js/polynomial-redos)", () => { + it("makes package slugs with no dash at either end or at the 64-character cut", () => { expect(slugify(" Hello, World! ")).toBe("hello-world"); expect(slugify("---")).toBe("site"); expect(slugify("a".repeat(70))).toBe("a".repeat(64)); + // The regex version this replaced cut after the dash and returned 63 a's and a "-". expect(slugify(`${"a".repeat(63)} b`)).toBe("a".repeat(63)); - const start = performance.now(); + // Long separator runs: behaviour only. The regex version was linear here too (its first + // replace collapsed every run before the trim), so no input tells the two apart by time + // (#168 item 10); the loop is for CodeQL js/polynomial-redos. expect(slugify("-".repeat(100_000))).toBe("site"); expect(slugify(`x${" -".repeat(100_000)}y`)).toBe("x-y"); + }); + + it("finds the setup fragment in linear time (CodeQL js/polynomial-redos)", () => { + const start = performance.now(); const long = describeSetup({ url: `http://localhost:8787/login#setup=${"#setup=".repeat(100_000)}`, tokenSource: "env", diff --git a/packages/core-ts/src/media/types.ts b/packages/core-ts/src/media/types.ts index b1f0d159..30432cd7 100644 --- a/packages/core-ts/src/media/types.ts +++ b/packages/core-ts/src/media/types.ts @@ -4,6 +4,7 @@ * so a client can never pick a kind or an extension (invariant 4). Worker-safe. */ import type { Asset } from "@cloudhal/core-contracts"; +import { trimChar } from "../names.ts"; export type AssetKind = Asset["kind"]; @@ -248,14 +249,12 @@ const SPECIAL: Record = { /** A URL slug from a file name or title: `Boğaz at Dawn (1).JPG` → `bogaz-at-dawn-1`. */ export function slugifyFilename(name: string | undefined, fallback = "file"): string { const base = (name ?? "").replace(/\.[A-Za-z0-9]{1,10}$/, ""); - const slug = base + const dashed = base .toLowerCase() .replace(/[ıßæœøđðłþ]/g, (c) => SPECIAL[c] ?? c) .normalize("NFKD") .replace(/[̀-ͯ]/g, "") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 96) - .replace(/-+$/g, ""); + .replace(/[^a-z0-9]+/g, "-"); + const slug = trimChar(trimChar(dashed, "-").slice(0, 96), "-"); return slug || fallback; } diff --git a/packages/core-ts/src/names.ts b/packages/core-ts/src/names.ts index c42054b1..9cd3929c 100644 --- a/packages/core-ts/src/names.ts +++ b/packages/core-ts/src/names.ts @@ -27,13 +27,26 @@ const SPECIAL: Record = { ð: "d", }; +/** + * `text` without `ch` (one UTF-16 unit, e.g. `-`) at either end, in one linear pass. Use it in + * place of a `/^-+|-+$/g` trim: CodeQL reports that pattern as polynomial backtracking + * (js/polynomial-redos) on long runs of `ch`. + */ +export function trimChar(text: string, ch: string): string { + let start = 0; + let end = text.length; + while (start < end && text[start] === ch) start++; + while (end > start && text[end - 1] === ch) end--; + return start === 0 && end === text.length ? text : text.slice(start, end); +} + /** * Folds a name for comparison: NFKC, Turkish-aware lower-casing (İ, I and ı all become i), * diacritics removed, apostrophes and dots dropped (`A.I.` → `ai`), every other run of * non-letters/digits turned into a single `-`. `Yapay Zekâ` → `yapay-zeka`; a folded slug is itself. */ export function foldName(input: string): string { - return input + const dashed = input .normalize("NFKC") .replace(/[İIı]/g, "i") .toLowerCase() @@ -41,8 +54,8 @@ export function foldName(input: string): string { .replace(/\p{M}+/gu, "") .replace(/[øßæœđłþð]/g, (c) => SPECIAL[c] ?? c) .replace(/['’‘`ʼ.]/g, "") - .replace(/[^\p{L}\p{N}]+/gu, "-") - .replace(/^-+|-+$/g, ""); + .replace(/[^\p{L}\p{N}]+/gu, "-"); + return trimChar(dashed, "-"); } /** @@ -61,11 +74,8 @@ export function pluralForms(folded: string): string[] { /** A slug for a free-text name (`Machine Learning` → `machine-learning`); empty when nothing is left. */ export function slugify(name: string): string { - return foldName(name) - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 128) - .replace(/-+$/, ""); + const dashed = foldName(name).replace(/[^a-z0-9]+/g, "-"); + return trimChar(trimChar(dashed, "-").slice(0, 128), "-"); } /** `machine-learning` → `Machine learning`. */ diff --git a/packages/core-ts/test/media.test.ts b/packages/core-ts/test/media.test.ts index bb577bb8..dc483cfe 100644 --- a/packages/core-ts/test/media.test.ts +++ b/packages/core-ts/test/media.test.ts @@ -559,6 +559,10 @@ describe("references and URLs", () => { expect(slugifyFilename("Boğaz at Dawn (1).JPG")).toBe("bogaz-at-dawn-1"); expect(slugifyFilename("İstanbul ışıkları.png")).toBe("istanbul-isiklari"); expect(slugifyFilename("...")).toBe("file"); + // #168: linear trims, and a cut at 96 characters that lands on a dash drops it. + expect(slugifyFilename(`--${"-".repeat(100_000)}--.png`)).toBe("file"); + expect(slugifyFilename(`${"a".repeat(95)} b.jpg`)).toBe("a".repeat(95)); + expect(slugifyFilename(" -Hello- .txt")).toBe("hello"); }); }); diff --git a/packages/core-ts/test/names.test.ts b/packages/core-ts/test/names.test.ts index 51c9deea..c5317a43 100644 --- a/packages/core-ts/test/names.test.ts +++ b/packages/core-ts/test/names.test.ts @@ -7,6 +7,7 @@ import { type ResolvableTerm, resolveTerm, slugify, + trimChar, } from "../src/names.ts"; const term = ( @@ -49,6 +50,33 @@ describe("foldName", () => { expect(foldName("machine-learning")).toBe("machine-learning"); }); + it("trimChar trims one character at both ends, linearly", () => { + expect(trimChar("--a-b--", "-")).toBe("a-b"); + expect(trimChar("a-b", "-")).toBe("a-b"); + expect(trimChar("----", "-")).toBe(""); + expect(trimChar("", "-")).toBe(""); + expect(trimChar("__x_", "_")).toBe("x"); + expect(trimChar(`a${"-".repeat(200_000)}b`, "-")).toBe(`a${"-".repeat(200_000)}b`); + }); + + it("slugify trims dashes at both ends and at the 128-character cut (#168)", () => { + expect(slugify(" --Hello, World!-- ")).toBe("hello-world"); + expect(slugify("---")).toBe(""); + expect(slugify("-".repeat(100_000))).toBe(""); + // A cut that lands on a dash drops it; the slug never ends in "-". + expect(slugify(`${"a".repeat(127)} b`)).toBe("a".repeat(127)); + expect(slugify("a".repeat(200))).toBe("a".repeat(128)); + // The same answers as the regex version it replaced (`/^-+|-+$/g`, then `/-+$/`). + const regex = (name: string) => + foldName(name) + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 128) + .replace(/-+$/, ""); + for (const input of ["", "A.I.", " x ", "a--b", "Çağrı & Öz", `${"ab ".repeat(60)}`, "--"]) + expect(slugify(input), input).toBe(regex(input)); + }); + it("slugify, humanizeSlug, pluralForms and editDistance", () => { expect(slugify("Yapay Zekâ & Robotlar")).toBe("yapay-zeka-robotlar"); expect(humanizeSlug("machine-learning")).toBe("Machine learning"); diff --git a/packages/storage/migrations/0030_rate_limits.ts b/packages/storage/migrations/0030_rate_limits.ts new file mode 100644 index 00000000..d9377a7b --- /dev/null +++ b/packages/storage/migrations/0030_rate_limits.ts @@ -0,0 +1,24 @@ +import type { Migration } from "../src/migrate.ts"; + +/** + * #168 item 8: Cloudhal's own fixed-window counters (`StaffStore.hit`: the hourly per-address + * reset-mail cap, the per-minute public token buckets, each agent key's window) leave Better + * Auth's `rateLimit` table (0005). Better Auth prunes that table by its own 60 s window and so + * deleted the hourly counters too, which let the reset-mail cap restart every minute. + * + * `rate_limits`: one row per counter key (`cloudhal:…`), its `count` in the window that began at + * `window_start` (epoch ms). Nothing prunes it by age; a stale row is reset by the next hit. The + * old `cloudhal:` rows in `rateLimit` are deleted (Better Auth would prune them anyway), so a + * counter restarts once at upgrade. + */ +export const migration0030RateLimits: Migration = { + name: "0030_rate_limits", + sql: ` +CREATE TABLE rate_limits ( + key TEXT NOT NULL PRIMARY KEY, + count INTEGER NOT NULL, + window_start INTEGER NOT NULL +); +DELETE FROM "rateLimit" WHERE "key" LIKE 'cloudhal:%'; +`, +}; diff --git a/packages/storage/migrations/index.ts b/packages/storage/migrations/index.ts index 0104e059..765dc640 100644 --- a/packages/storage/migrations/index.ts +++ b/packages/storage/migrations/index.ts @@ -28,6 +28,7 @@ import { migration0026SearchCjk } from "./0026_search_cjk.ts"; import { migration0027MultiSite } from "./0027_multi_site.ts"; import { migration0028SiteMembers } from "./0028_site_members.ts"; import { migration0029EntryListValues } from "./0029_entry_list_values.ts"; +import { migration0030RateLimits } from "./0030_rate_limits.ts"; /** All Cloudhal storage migrations, in order. */ export const migrations: readonly Migration[] = [ @@ -60,4 +61,5 @@ export const migrations: readonly Migration[] = [ migration0027MultiSite, migration0028SiteMembers, migration0029EntryListValues, + migration0030RateLimits, ]; diff --git a/packages/storage/test/migrate.test.ts b/packages/storage/test/migrate.test.ts index e83454e7..1c029ba8 100644 --- a/packages/storage/test/migrate.test.ts +++ b/packages/storage/test/migrate.test.ts @@ -47,6 +47,7 @@ describe("migrate", () => { "0027_multi_site", "0028_site_members", "0029_entry_list_values", + "0030_rate_limits", ]); expect((await migrate(driver, migrations)).applied).toEqual([]); expect(await appliedMigrations(driver)).toEqual([ @@ -79,6 +80,7 @@ describe("migrate", () => { "0027_multi_site", "0028_site_members", "0029_entry_list_values", + "0030_rate_limits", ]); const tables = await driver.all<{ name: string }>( "SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name", @@ -124,6 +126,7 @@ describe("migrate", () => { "password_reset", "projects", "rateLimit", + "rate_limits", "release_items", "releases", "schedule_runs", @@ -190,6 +193,7 @@ describe("migrate", () => { "0027_multi_site", "0028_site_members", "0029_entry_list_values", + "0030_rate_limits", ]); expect(await appliedMigrations(driver)).toEqual([ "0001_init", @@ -221,6 +225,7 @@ describe("migrate", () => { "0027_multi_site", "0028_site_members", "0029_entry_list_values", + "0030_rate_limits", ]); await h.dispose(); }); @@ -259,6 +264,7 @@ describe("migrate", () => { "0027_multi_site", "0028_site_members", "0029_entry_list_values", + "0030_rate_limits", ]); const ok = await driver.get("SELECT name FROM sqlite_master WHERE name = 'ok'"); expect(ok).toBeUndefined(); From f137899ccb37d49ab5a76596612d3a0f39e9bc27 Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:42:32 +0300 Subject: [PATCH 2/2] fix: review round 2 for #168 (rate-limit pruning, step-up loops, ReDoS) - rate_limits: expires_at with an index; one upsert RETURNING count per hit; about one hit in 32 prunes up to 500 expired counters. Migration 0030 edited in place (unreleased). Storage conformance proves get returns RETURNING rows on better-sqlite3, D1 and do-sqlite. - stepUpScopes: asks only for scopes both the site role and the installation role give (consent caps by the installation role), and never when the call would still be denied after consent (plan mode, an uncovering skill). - Linear trims for surface-interop ids and for input-reachable trailing trims: request paths (playground guard, route matching, analytics, renderer links, docs Worker), directive sources, comment lines; new trimEndChar in @cloudhal/core-ts/names. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/linear-trims-168.md | 3 +- .changeset/mcp-step-up-168.md | 2 +- .changeset/rate-limits-168.md | 2 +- apps/docs/src/worker/paths.ts | 10 ++-- docs/rfcs/0044-mcp-oauth-authorization.md | 9 +++- packages/agent-runtime/src/step-up.ts | 33 +++++++----- packages/agent-runtime/test/step-up.test.ts | 50 ++++++++++++++++++- packages/analytics/src/paths.ts | 5 +- packages/api/src/playground.ts | 3 +- packages/api/src/rate-limits.ts | 45 +++++++++++++++++ packages/api/src/scopes.ts | 3 +- packages/api/src/staff.ts | 26 ++++------ packages/api/src/testing-mcp-oauth.ts | 17 ++++++- packages/api/src/testing-rate-limits.ts | 46 +++++++++++++++++ packages/api/test/playground.test.ts | 8 +++ packages/astro-renderer/src/feeds.ts | 3 +- packages/astro-renderer/src/locales.ts | 3 +- packages/comments/src/normalize.ts | 25 ++++++---- packages/comments/test/text.test.ts | 8 +++ packages/core-ts/src/directives.ts | 4 +- packages/core-ts/src/names.ts | 7 +++ packages/core-ts/src/surface-interop.ts | 16 +++--- packages/core-ts/test/directives.test.ts | 8 +++ packages/core-ts/test/interop.test.ts | 19 +++++++ packages/core-ts/test/names.test.ts | 11 ++++ .../storage/migrations/0030_rate_limits.ts | 12 +++-- packages/storage/src/testing.ts | 11 ++++ scripts/size-baseline.json | 2 +- 28 files changed, 319 insertions(+), 72 deletions(-) create mode 100644 packages/api/src/rate-limits.ts diff --git a/.changeset/linear-trims-168.md b/.changeset/linear-trims-168.md index d2871dab..bcaa22a6 100644 --- a/.changeset/linear-trims-168.md +++ b/.changeset/linear-trims-168.md @@ -3,8 +3,9 @@ "@cloudhal/analytics": patch "@cloudhal/astro-renderer": patch "@cloudhal/cli": patch +"@cloudhal/comments": patch --- -Slug and name trims run in one linear pass instead of `/^-+|-+$/g` (CodeQL js/polynomial-redos, #168). This covers `slugify`, `foldName` and `slugifyFilename` in core-ts (the new `trimChar` is exported from `@cloudhal/core-ts/names`), heading ids in astro-renderer, and Studio's option, media and entry slugs. Results are unchanged. These trims were already linear in practice, because a run of other characters was collapsed to one `-` first. `datasetName` in analytics was quadratic on a long run of `_` inside a deployment name; it is linear now too. +Slug and name trims run in one linear pass instead of `/^-+|-+$/g` (CodeQL js/polynomial-redos, #168). This covers `slugify`, `foldName` and `slugifyFilename` in core-ts (the new `trimChar` is exported from `@cloudhal/core-ts/names`), heading ids in astro-renderer, and Studio's option, media and entry slugs. Results are unchanged. These trims were already linear in practice, because a run of other characters was collapsed to one `-` first. These inputs were quadratic and are linear now: analytics' `datasetName` on a long run of `_` inside a deployment name; json-render component keys in `fromJsonRender` (`./interop`); trailing whitespace in directive blocks and on comment lines; trailing `/` in request paths (the playground guard and route matching in api, analytics paths, the renderer's link localizing and redirect splats, the docs Worker). The new `trimEndChar` is exported from `@cloudhal/core-ts/names` too. Also in 0.6.3, with no changeset at the time (#161): Studio's "Not published" badges and its "unknown" scheduler state show their status dot again. They used a Badge variant without a dot colour and rendered no dot. diff --git a/.changeset/mcp-step-up-168.md b/.changeset/mcp-step-up-168.md index 0ef66c08..b0ffe187 100644 --- a/.changeset/mcp-step-up-168.md +++ b/.changeset/mcp-step-up-168.md @@ -3,4 +3,4 @@ "@cloudhal/api": patch --- -MCP OAuth step-up follow-ups (RFC-0044 phase 1, #168). These stay latent until phase 2 gives connections a `grantId`. A policy denial for missing scopes now carries `missing`, the required scopes that neither the caller's scopes nor an own variant satisfy. The new `stepUpScopes(policy, principal, tool, args)` turns that into the scopes a token should ask for, using the same rules as the handler: a propose call's `proposeScopes`, the own variant the member's role holds (`content:write:own` for an Author, `content:draft:own` for a Contributor, `site:publish:own` for an Author's publish) and the member's role on the call's site (RFC-0034 A2), not their installation role. The `/mcp` step-up 403 asks for exactly that set and now leaves an audit row, like other denials. `createApi` throws when `mcp.oauth.resource` is not the public URL of `/mcp`. A playground refuses both OAuth protected-resource documents (`403 {error: "playground", block: "mcp"}`), as its route table already said. +MCP OAuth step-up follow-ups (RFC-0044 phase 1, #168). These stay latent until phase 2 gives connections a `grantId`. A policy denial for missing scopes now carries `missing`, the required scopes that neither the caller's scopes nor an own variant satisfy. The new `stepUpScopes(policy, principal, tool, args)` turns that into the scopes a token should ask for, using the same rules as the handler: a propose call's `proposeScopes`, the own variant the member's role holds (`content:write:own` for an Author, `content:draft:own` for a Contributor, `site:publish:own` for an Author's publish) and the member's role on the call's site (RFC-0034 A2), capped by their installation role as consent caps a token (an installation Contributor who is an Editor on one site is asked for `content:draft:own` there). A call that would still be denied with those scopes, such as a write in plan mode or a tool outside the declared skill, is not challenged. The `/mcp` step-up 403 asks for exactly that set and now leaves an audit row, like other denials. `createApi` throws when `mcp.oauth.resource` is not the public URL of `/mcp`. A playground refuses both OAuth protected-resource documents (`403 {error: "playground", block: "mcp"}`), as its route table already said. diff --git a/.changeset/rate-limits-168.md b/.changeset/rate-limits-168.md index 6e42e4fc..9c2e7eb5 100644 --- a/.changeset/rate-limits-168.md +++ b/.changeset/rate-limits-168.md @@ -3,4 +3,4 @@ "@cloudhal/api": patch --- -Cloudhal's own rate-limit counters move out of Better Auth's `rateLimit` table into `rate_limits` (migration `0030_rate_limits`, #168). This covers the hourly cap of 3 reset mails per address, the per-minute buckets of the public token endpoints and each agent key's window. Better Auth prunes `rateLimit` by its own 60 s window, so the hourly reset-mail cap used to restart about a minute after any sign-in from the same address. The migration deletes the old `cloudhal:` rows from `rateLimit`, so each counter starts over once at upgrade. +Cloudhal's own rate-limit counters move out of Better Auth's `rateLimit` table into `rate_limits` (migration `0030_rate_limits`, #168). This covers the hourly cap of 3 reset mails per address, the per-minute buckets of the public token endpoints and each agent key's window. Better Auth prunes `rateLimit` by its own 60 s window, so the hourly reset-mail cap used to restart about a minute after any sign-in from the same address. Each hit counts in one statement (an upsert with `RETURNING`). Because the keys come from client addresses and submitted email addresses, about one hit in 32 also deletes up to 500 expired counters through an `expires_at` index. The migration deletes the old `cloudhal:` rows from `rateLimit`, so each counter starts over once at upgrade. diff --git a/apps/docs/src/worker/paths.ts b/apps/docs/src/worker/paths.ts index fa526857..fa4e7c9b 100644 --- a/apps/docs/src/worker/paths.ts +++ b/apps/docs/src/worker/paths.ts @@ -22,13 +22,15 @@ export function pageId(input: string, selfHost?: string): string | undefined { if (!HOSTS.has(url.hostname) && url.host !== selfHost) return undefined; value = url.pathname; } - const path = value + const stem = value .replace(/[?#].*$/, "") .replace(/^\/+/, "") .replace(/(^|\/)index\.md$/, "") - .replace(/\.md$/, "") - .replace(/\/+$/, "") - .toLowerCase(); + .replace(/\.md$/, ""); + // A linear trim of trailing "/" (`/\/+$/` backtracks on a long run of "/" inside the path). + let end = stem.length; + while (end > 0 && stem[end - 1] === "/") end--; + const path = stem.slice(0, end).toLowerCase(); if (path === "" || path === "index") return "index"; if (!ID.test(path) || path.split("/").some((s) => s === "." || s === "..")) return undefined; return path; diff --git a/docs/rfcs/0044-mcp-oauth-authorization.md b/docs/rfcs/0044-mcp-oauth-authorization.md index 60fb7ad3..f8101ce7 100644 --- a/docs/rfcs/0044-mcp-oauth-authorization.md +++ b/docs/rfcs/0044-mcp-oauth-authorization.md @@ -163,8 +163,13 @@ arguments of the single message a stateless request carries and asks the policy the same rules as the handler's decision: a propose call needs its `proposeScopes`, an own variant satisfies a tool that allows one (an Author is asked for `site:publish:own`, a Contributor for `content:draft:own`), and the member's role is their role on the call's site -(RFC-0034 A2), not their installation role. Every other denial (role, mode, `:own`, approvals) -stays a JSON-RPC result as today. The 403 leaves an audit row like every other denial. Clients then +(RFC-0034 A2). Only scopes both that site role and the installation role give are asked for, +because consent caps a token by the installation role and `forSite` then caps it by the site +role: an installation Contributor who is an Editor on one site is asked for `content:draft:own` +there, not `content:write`. No challenge is sent when the call would still be denied once the +token has those scopes (plan mode, a skill that does not cover the tool). Every other denial +(role, mode, `:own`, approvals) stays a JSON-RPC result as today. The 403 leaves an audit row +like every other denial. Clients then re-authorize with the union of their scopes (the SDK's `computeScopeUnion`; the draft revision makes the union normative). A scope the role does not hold is never challenged: the client would loop. The challenge is emitted together for all scopes the tool needs ("servers SHOULD include all diff --git a/packages/agent-runtime/src/step-up.ts b/packages/agent-runtime/src/step-up.ts index 4a0735a4..e6c7c637 100644 --- a/packages/agent-runtime/src/step-up.ts +++ b/packages/agent-runtime/src/step-up.ts @@ -12,15 +12,17 @@ import { type Principal, type Scope, } from "./policy.ts"; -import { roleScopes } from "./roles.ts"; +import { narrow, roleScopes } from "./roles.ts"; import { forInstallation, forSite, INSTALLATION_SCOPES, isSiteLimited } from "./site-access.ts"; /** * The scopes to ask for so that `principal` may make the call `toolName(args)`, each the - * strongest one the member's role on the call's site gives a token (`site:publish:own` for an - * Author, not `site:publish`). Undefined when the call is not denied for a missing scope, when - * the principal has no person behind it, or when the role cannot give every missing scope (a - * challenge would then make the client loop). + * strongest one a token can hold on the call's site: given by the member's role there and by + * their installation role, which caps what consent issues (`site:publish:own` for an Author, not + * `site:publish`). Undefined when the call is not denied for a missing scope, when the principal + * has no person behind it, when the roles cannot give every missing scope, or when the call + * would still be denied with them (plan mode, a skill that does not cover it): a challenge would + * then make the client re-authorize for nothing, or loop. */ export function stepUpScopes( policy: PolicyEngine, @@ -32,15 +34,19 @@ export function stepUpScopes( if (!tool || !principal.person) return undefined; // The handler judges the call as the principal acts on its site (`handleToolCall`). const siteId = typeof args.siteId === "string" ? args.siteId : undefined; - const sited = siteId ? forSite(principal, siteId) : forInstallation(principal); + const onSite = (p: Principal) => (siteId ? forSite(p, siteId) : forInstallation(p)); + const sited = onSite(principal); const person = sited?.person; if (!sited || !person) return undefined; const decision = policy.evaluate(sited, toolName, { args }); if (decision.kind !== "deny" || decision.code !== "missing_scope" || !decision.missing?.length) return undefined; - const role = roleScopes(person.role); + // Consent caps a token by the installation role, as key creation does; `forSite` then caps it + // by the site role. Only scopes both give survive, so only those are asked for. + const installation = roleScopes(principal.person.role); + const both = narrow(roleScopes(person.role), installation); const grantable = - !siteId && isSiteLimited(person) ? role.filter((s) => !INSTALLATION_SCOPES.includes(s)) : role; + !siteId && isSiteLimited(person) ? both.filter((s) => !INSTALLATION_SCOPES.includes(s)) : both; const asked: Scope[] = []; for (const scope of decision.missing) { const pick = [scope, ...ownVariantsOf(tool, scope)].find( @@ -49,9 +55,10 @@ export function stepUpScopes( if (!pick) return undefined; if (!asked.includes(pick)) asked.push(pick); } - // With them the scope check must pass, or the client would re-authorize for nothing. - const after = policy.evaluate({ ...sited, scopes: [...sited.scopes, ...asked] }, toolName, { - args, - }); - return after.kind === "deny" && after.code === "missing_scope" ? undefined : asked; + // The call as it would run after consent must not be denied for any reason (scope, mode, + // skill): otherwise the client would re-authorize for nothing, or loop. + const granted = narrow([...(principal.keyScopes ?? principal.scopes), ...asked], installation); + const after = onSite({ ...principal, scopes: granted, keyScopes: granted }); + if (!after) return undefined; + return policy.evaluate(after, toolName, { args }).kind === "deny" ? undefined : asked; } diff --git a/packages/agent-runtime/test/step-up.test.ts b/packages/agent-runtime/test/step-up.test.ts index 759dcff9..28f22002 100644 --- a/packages/agent-runtime/test/step-up.test.ts +++ b/packages/agent-runtime/test/step-up.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vitest"; +import { scopesForMode } from "../src/modes.ts"; import { type PersonRef, PolicyEngine, type Principal, type Scope } from "../src/policy.ts"; -import type { Role } from "../src/roles.ts"; +import { narrow, type Role, roleScopes } from "../src/roles.ts"; +import { forSite } from "../src/site-access.ts"; import { stepUpScopes } from "../src/step-up.ts"; const engine = new PolicyEngine([ @@ -95,6 +97,52 @@ describe("stepUpScopes (RFC-0044 §1)", () => { expect(stepUpScopes(engine, limited, "update_entry", { siteId: "site_c" })).toBeUndefined(); }); + it("asks only for what both the site role and the installation role give (no loop)", () => { + // An installation Contributor who is an Editor on site_b: consent caps the token at the + // Contributor's scopes, so asking for content:write would come back as content:draft:own. + const contributor = grant("contributor", READ, { site_b: "editor" }); + const args = { siteId: "site_b" }; + const asked = stepUpScopes(engine, contributor, "update_entry", args); + expect(asked).toEqual(["content:draft:own"]); + // After consent (capped by the installation role) the call runs and nothing is asked again. + const granted = narrow([...READ, ...(asked ?? [])], roleScopes("contributor")); + const after = { ...contributor, scopes: granted, keyScopes: granted }; + expect(stepUpScopes(engine, after, "update_entry", args)).toBeUndefined(); + const onSite = forSite(after, "site_b"); + expect(onSite && engine.evaluate(onSite, "update_entry", { args })).toMatchObject({ + kind: "allow", + limit: { level: "draft" }, + }); + // Publishing is neither role's to give on that site through a Contributor's consent. + expect(stepUpScopes(engine, contributor, "publish", args)).toBeUndefined(); + }); + + it("never challenges a call that would still be denied after consent", () => { + // Plan mode: the token lacks content:write, but plan mode refuses the write anyway. + const plan = { ...grant("editor", scopesForMode("plan")), mode: "plan" as const }; + expect(engine.evaluate(plan, "update_entry")).toMatchObject({ code: "missing_scope" }); + expect(stepUpScopes(engine, plan, "update_entry")).toBeUndefined(); + // ... while a propose call, which plan mode may make, is still challenged when it lacks scopes. + expect( + stepUpScopes( + engine, + { ...plan, scopes: ["site:read"], keyScopes: ["site:read"] }, + "edit_body", + { + mode: "propose", + }, + ), + ).toEqual(["content:read", "content:propose"]); + // A declared skill that does not cover the tool's scope: the skill check would refuse it. + const skilled = { + ...grant("editor", READ), + skill: { name: "reader", scopes: ["site:read", "content:read"], approval: [] }, + }; + expect(stepUpScopes(engine, skilled, "update_entry")).toBeUndefined(); + const covering = { ...skilled, skill: { ...skilled.skill, scopes: ["content:write"] } }; + expect(stepUpScopes(engine, covering, "update_entry")).toEqual(["content:write"]); + }); + it("never asks for a person-only scope, and never for a call that is not a scope denial", () => { expect(stepUpScopes(engine, grant("administrator", READ), "audit_log")).toBeUndefined(); const writer = grant("editor", [...READ, "content:write"]); diff --git a/packages/analytics/src/paths.ts b/packages/analytics/src/paths.ts index 4d08a145..f2a773f0 100644 --- a/packages/analytics/src/paths.ts +++ b/packages/analytics/src/paths.ts @@ -27,7 +27,10 @@ export function normaliseBeaconPath(raw: string): string | undefined { return undefined; } // no query split after decoding: an encoded "?" is part of the path, and the route pattern refuses it - const trimmed = decoded.length > 1 ? decoded.replace(/\/+$/, "") || "/" : decoded; + // A linear trim of trailing "/" (`/\/+$/` backtracks on a long run of "/" inside the path). + let end = decoded.length; + while (end > 1 && decoded[end - 1] === "/") end--; + const trimmed = decoded.slice(0, end); const path = trimmed.toLowerCase(); return ROUTE_PATH_RE.test(path) ? path : undefined; } diff --git a/packages/api/src/playground.ts b/packages/api/src/playground.ts index 2997a574..79779865 100644 --- a/packages/api/src/playground.ts +++ b/packages/api/src/playground.ts @@ -8,6 +8,7 @@ * refused as `unclassified`, so a new route is never silently open in a playground. */ import { type Principal, ROLE_SCOPES, type Scope } from "@cloudhal/agent-runtime"; +import { trimEndChar } from "@cloudhal/core-ts/names"; import { createMiddleware } from "hono/factory"; import type { Env } from "./context.ts"; import { matchesRoute, routeFor } from "./scopes.ts"; @@ -220,7 +221,7 @@ const WELL_KNOWN_API = "/.well-known/oauth-protected-resource"; * the rendered site) is not the API's. */ export function isBlockedInPlayground(method: string, path: string): PlaygroundBlock | undefined { - const clean = path.replace(/\/+$/, "") || "/"; + const clean = trimEndChar(path, "/") || "/"; if (clean === "/mcp" || clean.startsWith("/mcp/")) return "mcp"; const wellKnown = clean === WELL_KNOWN_API || clean.startsWith(`${WELL_KNOWN_API}/`); if (!clean.startsWith("/api/") && !wellKnown) return undefined; diff --git a/packages/api/src/rate-limits.ts b/packages/api/src/rate-limits.ts new file mode 100644 index 00000000..950ae685 --- /dev/null +++ b/packages/api/src/rate-limits.ts @@ -0,0 +1,45 @@ +/** + * Cloudhal's own fixed-window counters (#168 item 8), in `rate_limits` (migration 0030) rather + * than Better Auth's `rateLimit`, which Better Auth prunes by its own 60 s window. + * + * Counting is one statement (an upsert with `RETURNING`), so two hits never read each other's + * half-done window. The table is pruned here, because nothing else does and its keys come from + * client addresses and submitted email addresses: about one hit in `PRUNE_EVERY` also deletes up + * to `PRUNE_LIMIT` expired rows through the `expires_at` index. The prune is a statement of its + * own (a `batch()` returns no rows, so it cannot carry the count), and it may run before or after + * any hit: it only removes rows whose window has ended, which the next hit on that key would + * reset to 1 anyway. + */ +import type { SqlDriver } from "@cloudhal/storage"; + +/** On average one hit in this many prunes expired counters. */ +export const PRUNE_EVERY = 32; +/** The most expired rows one prune deletes (D1 prefers short statements). */ +export const PRUNE_LIMIT = 500; + +/** The default prune decision: random, so per-request instances (Workers) share the cost. */ +export const randomPrune = (): boolean => Math.random() < 1 / PRUNE_EVERY; + +/** Counts one hit on `key`; the result is the count in the current window, this hit included. */ +export async function countHit( + sql: SqlDriver, + hit: { key: string; windowMs: number; now: number }, +): Promise { + const { key, windowMs, now } = hit; + // In an UPDATE every column reference is the row before the update (SQLite), so the three + // CASEs agree on whether the window has ended. + const row = await sql.get<{ count: number }>( + "INSERT INTO rate_limits (key, count, window_start, expires_at) VALUES (?, 1, ?, ?) ON CONFLICT (key) DO UPDATE SET count = CASE WHEN rate_limits.expires_at < ? THEN 1 ELSE rate_limits.count + 1 END, window_start = CASE WHEN rate_limits.expires_at < ? THEN excluded.window_start ELSE rate_limits.window_start END, expires_at = CASE WHEN rate_limits.expires_at < ? THEN excluded.expires_at ELSE rate_limits.expires_at END RETURNING count", + [key, now, now + windowMs, now, now, now], + ); + return Number(row?.count ?? 0); +} + +/** Deletes up to `PRUNE_LIMIT` counters whose window ended before `now`; returns how many. */ +export async function pruneCounters(sql: SqlDriver, now: number): Promise { + const r = await sql.run( + "DELETE FROM rate_limits WHERE rowid IN (SELECT rowid FROM rate_limits WHERE expires_at < ? LIMIT ?)", + [now, PRUNE_LIMIT], + ); + return r.changes; +} diff --git a/packages/api/src/scopes.ts b/packages/api/src/scopes.ts index 33848264..c850b4f1 100644 --- a/packages/api/src/scopes.ts +++ b/packages/api/src/scopes.ts @@ -14,6 +14,7 @@ * Agents (API keys) are refused everywhere except `public`/`signed-in`: they work through MCP. */ import { forInstallation, forSite, holds, type Scope } from "@cloudhal/agent-runtime"; +import { trimEndChar } from "@cloudhal/core-ts/names"; import { createMiddleware } from "hono/factory"; import type { Env } from "./context.ts"; import { apiError, missingScope, unauthorized } from "./errors.ts"; @@ -342,7 +343,7 @@ export const ROUTE_SCOPES: readonly (readonly [Method, string, RouteRequirement] /** Whether `path` fits a `ROUTE_SCOPES` pattern (`:param` one segment, `*` the rest). */ export function matchesRoute(pattern: string, path: string): boolean { const p = pattern.split("/"); - const s = path.replace(/\/+$/, "").split("/"); + const s = trimEndChar(path, "/").split("/"); for (let i = 0; i < p.length; i++) { const part = p[i] as string; if (part === "*") return s.length > i; diff --git a/packages/api/src/staff.ts b/packages/api/src/staff.ts index 71534e89..89b81470 100644 --- a/packages/api/src/staff.ts +++ b/packages/api/src/staff.ts @@ -9,6 +9,7 @@ */ import { isRole, type Role } from "@cloudhal/agent-runtime"; import type { SqlDriver } from "@cloudhal/storage"; +import { countHit, pruneCounters, randomPrune } from "./rate-limits.ts"; import { allSiteRoles, inviteSiteRoles, @@ -91,10 +92,13 @@ function toMember(r: Row): MemberRow | undefined { export class StaffStore { readonly #sql: SqlDriver; readonly #now: () => Date; + readonly #prune: () => boolean; - constructor(sql: SqlDriver, now: () => Date = () => new Date()) { + /** `prune` decides per hit whether to delete expired rate-limit counters (tests pin it). */ + constructor(sql: SqlDriver, now: () => Date = () => new Date(), prune = randomPrune) { this.#sql = sql; this.#now = now; + this.#prune = prune; } // ───────────────────────────── members ───────────────────────────── @@ -534,23 +538,15 @@ export class StaffStore { // ───────────────────────────── rate limits ───────────────────────────── /** - * Fixed-window counter under keys of our own (`cloudhal:…`), in Cloudhal's `rate_limits` table - * (migration 0030), not Better Auth's `rateLimit`: Better Auth prunes that by its own 60 s - * window, which reset hourly counters every minute (#168). One upsert, so concurrent hits never - * lose a count. Returns false once `max` requests were seen within `windowMs`. + * Fixed-window counter under keys of our own (`cloudhal:…`) in `rate_limits` (`./rate-limits.ts`, + * #168): one atomic upsert per hit, and now and then a prune of expired counters. Returns false + * once `max` requests were seen within `windowMs`. */ async hit(key: string, windowMs: number, max: number): Promise { const now = this.#now().getTime(); - const start = now - windowMs; - await this.#sql.run( - "INSERT INTO rate_limits (key, count, window_start) VALUES (?, 1, ?) ON CONFLICT (key) DO UPDATE SET count = CASE WHEN rate_limits.window_start < ? THEN 1 ELSE rate_limits.count + 1 END, window_start = CASE WHEN rate_limits.window_start < ? THEN excluded.window_start ELSE rate_limits.window_start END", - [key, now, start, start], - ); - const row = await this.#sql.get<{ count: number }>( - "SELECT count FROM rate_limits WHERE key = ?", - [key], - ); - return Number(row?.count ?? 0) <= max; + const count = await countHit(this.#sql, { key, windowMs, now }); + if (this.#prune()) await pruneCounters(this.#sql, now); + return count <= max; } // ───────────────────────────── helpers ───────────────────────────── diff --git a/packages/api/src/testing-mcp-oauth.ts b/packages/api/src/testing-mcp-oauth.ts index 23b972ef..6dfddbfc 100644 --- a/packages/api/src/testing-mcp-oauth.ts +++ b/packages/api/src/testing-mcp-oauth.ts @@ -245,15 +245,28 @@ export function describeMcpOauthDiscovery(label: string, backend: RolesBackend): const entry = await app.request(`${ORIGIN}/mcp`, toolCall("create_entry", entryArgs(siteId))); expect(entry.status).toBe(403); expect(await entry.json()).toMatchObject({ details: { scope: ["content:draft:own"] } }); - // Where the same member is an Editor, the challenge names content:write. + // An installation Contributor who is an Editor here: consent caps the token at the + // Contributor's scopes, so the challenge names what it can issue (content:draft:own), not + // content:write, which would come back narrowed and loop. With it the call goes through. const editorOn = (siteId: string) => grantPrincipal("contributor", read, { [siteId]: "editor" }); - expect(await challenged(editorOn, "create_entry", entryArgs)).toEqual(["content:write"]); + expect(await challenged(editorOn, "create_entry", entryArgs)).toEqual(["content:draft:own"]); + const consented = (siteId: string) => + grantPrincipal("contributor", [...read, "content:draft:own"], { [siteId]: "editor" }); + expect(await challenged(consented, "create_entry", entryArgs)).toBeUndefined(); // A site the member cannot open answers not_found from the handler, never a challenge. const elsewhere = grantPrincipal("editor", read, { site_other: "editor" }); expect(await challenged(elsewhere, "create_entry", entryArgs)).toBeUndefined(); }); + it("never challenges a plan-mode token for a write plan mode refuses anyway", async () => { + const plan: Principal = { + ...grantPrincipal("editor", scopesForMode("plan")), + mode: "plan", + }; + expect(await challenged(plan, "create_entry", entryArgs)).toBeUndefined(); + }); + it("records the step-up 403 in the audit log, like any other denial", async () => { const { app, siteId, audit } = await setupWithPrincipal( grantPrincipal("editor", ["site:read", "content:read"]), diff --git a/packages/api/src/testing-rate-limits.ts b/packages/api/src/testing-rate-limits.ts index 1f224482..d4111509 100644 --- a/packages/api/src/testing-rate-limits.ts +++ b/packages/api/src/testing-rate-limits.ts @@ -10,6 +10,8 @@ import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; import { createApi } from "./app.ts"; import { createAuth } from "./auth.ts"; import type { MailMessage } from "./mail.ts"; +import { PRUNE_LIMIT } from "./rate-limits.ts"; +import { StaffStore } from "./staff.ts"; import type { RolesBackend } from "./testing-roles.ts"; const ORIGIN = "http://limits.test"; @@ -94,5 +96,49 @@ export function describeRateLimitCounters(label: string, backend: RolesBackend): vi.setSystemTime(T0 + 61 * 60_000); expect(await askForResets(1)).toBe(4); }); + + it("prunes expired counters, which nothing else deletes", async () => { + const b = await backend.create(); + let now = T0; + let prune = false; + const staff = new StaffStore( + b.sql, + () => new Date(now), + () => prune, + ); + const keys = async () => + (await b.sql.all<{ key: string }>("SELECT key FROM rate_limits ORDER BY key")).map( + (r) => r.key, + ); + expect(await staff.hit("cloudhal:minute", 60_000, 3)).toBe(true); + expect(await staff.hit("cloudhal:hour", 3_600_000, 2)).toBe(true); + now = T0 + 61_000; + // A hit that does not prune leaves the stale row; a pruning one removes only expired rows. + expect(await staff.hit("cloudhal:other", 60_000, 3)).toBe(true); + expect(await keys()).toEqual(["cloudhal:hour", "cloudhal:minute", "cloudhal:other"]); + prune = true; + expect(await staff.hit("cloudhal:hour", 3_600_000, 2)).toBe(true); + expect(await keys()).toEqual(["cloudhal:hour", "cloudhal:other"]); + // The hourly counter kept its window: the third hit within the hour is over its max of 2. + expect(await staff.hit("cloudhal:hour", 3_600_000, 2)).toBe(false); + // A counter whose row was pruned starts again at 1. + expect(await staff.hit("cloudhal:minute", 60_000, 1)).toBe(true); + // One prune deletes at most PRUNE_LIMIT expired rows; later hits take the rest. + await b.sql.batch( + Array.from({ length: PRUNE_LIMIT + 5 }, (_, i) => ({ + sql: "INSERT INTO rate_limits (key, count, window_start, expires_at) VALUES (?, 1, 0, 1)", + params: [`cloudhal:stale:${i}`], + })), + ); + await staff.hit("cloudhal:other", 60_000, 3); + const left = await b.sql.get<{ n: number }>( + "SELECT count(*) AS n FROM rate_limits WHERE key LIKE 'cloudhal:stale:%'", + ); + expect(Number(left?.n)).toBe(5); + const index = await b.sql.get<{ name: string }>( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'rate_limits' AND name = 'rate_limits_expires'", + ); + expect(index?.name).toBe("rate_limits_expires"); + }); }); } diff --git a/packages/api/test/playground.test.ts b/packages/api/test/playground.test.ts index a22f3122..08202fa1 100644 --- a/packages/api/test/playground.test.ts +++ b/packages/api/test/playground.test.ts @@ -167,6 +167,14 @@ describe("createApi({ playground })", () => { expect(instance.features.mcp).toBe(false); }); + it("judges a path with a long run of slashes in linear time (#168)", () => { + // `/\/+$/` on the request path took about 150 ms per 16k slashes, on every request. + const start = performance.now(); + expect(isBlockedInPlayground("GET", `/api/${"/".repeat(200_000)}x`)).toBe("unclassified"); + expect(isBlockedInPlayground("GET", "/api/me///")).toBeUndefined(); + expect(performance.now() - start).toBeLessThan(1500); + }); + it("refuses the OAuth discovery documents even when mcp.oauth is configured", async () => { const sql = await BetterSqlite3Driver.open(":memory:"); const store = new SiteGraphStore({ sql, blobs: new MemoryBlobStore() }); diff --git a/packages/astro-renderer/src/feeds.ts b/packages/astro-renderer/src/feeds.ts index b07b9135..26e32c51 100644 --- a/packages/astro-renderer/src/feeds.ts +++ b/packages/astro-renderer/src/feeds.ts @@ -5,6 +5,7 @@ */ import type { ContentEntry, Page, Redirect, Site } from "@cloudhal/core-contracts"; import { collectionSettings, entryPath } from "@cloudhal/core-ts"; +import { trimEndChar } from "@cloudhal/core-ts/names"; import type { ArchiveSummary, ArchiveView } from "./archive.ts"; import { markdownText, renderBody } from "./markdown.ts"; import type { AssetIndex } from "./media.ts"; @@ -214,7 +215,7 @@ export function redirectsFile(redirects: readonly Redirect[]): string { .sort((a, b) => b.from.length - a.from.length || a.from.localeCompare(b.from)); const splat = (to: string) => { const i = to.search(/[?#]/); - const head = (i < 0 ? to : to.slice(0, i)).replace(/\/+$/, ""); + const head = trimEndChar(i < 0 ? to : to.slice(0, i), "/"); return `${head}/:splat${i < 0 ? "" : to.slice(i)}`; }; const lines = [ diff --git a/packages/astro-renderer/src/locales.ts b/packages/astro-renderer/src/locales.ts index 1fe942ae..f44204ff 100644 --- a/packages/astro-renderer/src/locales.ts +++ b/packages/astro-renderer/src/locales.ts @@ -9,6 +9,7 @@ import type { ContentEntry, Page, Site, Taxonomy, Term } from "@cloudhal/core-contracts"; import { entryPath, type PermalinkSite, termSlug } from "@cloudhal/core-ts"; import { localeRouting, localeUrl, localizedPath } from "@cloudhal/core-ts/locale-paths"; +import { trimEndChar } from "@cloudhal/core-ts/names"; import { routableEntries } from "./entry-views.ts"; import type { HeadMeta } from "./seo.ts"; import type { GraphSource } from "./source.ts"; @@ -247,7 +248,7 @@ export function localizeLinks(html: string, localize: (path: string) => string): const cut = url.search(/[?#]/); const path = cut < 0 ? url : url.slice(0, cut); const suffix = cut < 0 ? "" : url.slice(cut); - return `${head}${localize(path.length > 1 ? path.replace(/\/+$/, "") : path)}${suffix}${tail}`; + return `${head}${localize(path.length > 1 ? trimEndChar(path, "/") : path)}${suffix}${tail}`; }; return html .replace(/(<(?:a|form)\b[^>]*?\s(?:href|action)=")(\/[^"]*)(")/gi, swap) diff --git a/packages/comments/src/normalize.ts b/packages/comments/src/normalize.ts index b4779a0e..55702e7b 100644 --- a/packages/comments/src/normalize.ts +++ b/packages/comments/src/normalize.ts @@ -57,17 +57,20 @@ export function cutCodePoints(s: string, max: number): string { } export function normalizeText(raw: string): string { - return raw - .normalize("NFC") - .replace(/\r\n?/g, "\n") - .replace(UNSAFE_IN_TEXT, "") - .replace(INVISIBLE_RE, "") - .replace(LONE_ZWJ_RE, "") - .split("\n") - .map((line) => line.replace(/[^\S\n]+$/u, "")) - .join("\n") - .replace(/\n{4,}/g, "\n\n\n") - .trim(); + return ( + raw + .normalize("NFC") + .replace(/\r\n?/g, "\n") + .replace(UNSAFE_IN_TEXT, "") + .replace(INVISIBLE_RE, "") + .replace(LONE_ZWJ_RE, "") + .split("\n") + // `trimEnd`, not `/[^\S\n]+$/u`: the regex backtracks on a long run of spaces inside a line. + .map((line) => line.trimEnd()) + .join("\n") + .replace(/\n{4,}/g, "\n\n\n") + .trim() + ); } export function normalizeName(raw: string): string { diff --git a/packages/comments/test/text.test.ts b/packages/comments/test/text.test.ts index 2f3b1e62..e02d691c 100644 --- a/packages/comments/test/text.test.ts +++ b/packages/comments/test/text.test.ts @@ -40,6 +40,14 @@ describe("normalising (§6.4)", () => { ); }); + it("trims line ends in linear time (#168, CodeQL js/polynomial-redos)", () => { + // `/[^\S\n]+$/u` took seconds on a long run of spaces inside one line. + const line = `a${" ".repeat(200_000)}b`; + const start = performance.now(); + expect(normalizeText(`${line} \nnext`)).toBe(`${line}\nnext`); + expect(performance.now() - start).toBeLessThan(1500); + }); + it("removes control characters but tab and line feed, C1 controls and bidi overrides", () => { const hostile = "a\u0000b\u0007c\u001Bd\u007Fe\u0085f" + diff --git a/packages/core-ts/src/directives.ts b/packages/core-ts/src/directives.ts index b9285aa8..217d1dcb 100644 --- a/packages/core-ts/src/directives.ts +++ b/packages/core-ts/src/directives.ts @@ -156,7 +156,7 @@ export function matchDirective(src: string): { directive: Directive; raw: string /** A whole block's source as a directive, or `undefined` when it is not exactly one. */ export function parseDirective(markdown: string): Directive | undefined { - const source = markdown.replace(/\s+$/, ""); + const source = markdown.trimEnd(); // not `/\s+$/`: it backtracks on inner whitespace runs const m = matchDirective(source); return m && m.raw === source ? m.directive : undefined; } @@ -199,7 +199,7 @@ export function writeDirective( ): string { const attrs = directiveAttrsText(props); if (content === undefined) return `::${name}${attrs}`; - const inner = content.replace(/\s+$/, ""); + const inner = content.trimEnd(); let deepest = 2; for (const m of inner.matchAll(/^(:{3,})/gm)) deepest = Math.max(deepest, (m[1] as string).length); diff --git a/packages/core-ts/src/names.ts b/packages/core-ts/src/names.ts index 9cd3929c..0c0d6e76 100644 --- a/packages/core-ts/src/names.ts +++ b/packages/core-ts/src/names.ts @@ -40,6 +40,13 @@ export function trimChar(text: string, ch: string): string { return start === 0 && end === text.length ? text : text.slice(start, end); } +/** `text` without `ch` at its end, linearly: the `/\/+$/` kind of trim, for paths and the like. */ +export function trimEndChar(text: string, ch: string): string { + let end = text.length; + while (end > 0 && text[end - 1] === ch) end--; + return end === text.length ? text : text.slice(0, end); +} + /** * Folds a name for comparison: NFKC, Turkish-aware lower-casing (İ, I and ı all become i), * diacritics removed, apostrophes and dots dropped (`A.I.` → `ai`), every other run of diff --git a/packages/core-ts/src/surface-interop.ts b/packages/core-ts/src/surface-interop.ts index 1aa20715..a67ad64f 100644 --- a/packages/core-ts/src/surface-interop.ts +++ b/packages/core-ts/src/surface-interop.ts @@ -13,6 +13,7 @@ */ import type { SurfaceComponent, SurfaceSection } from "@cloudhal/core-contracts"; import { nearestIcon } from "./icon-names.ts"; +import { trimChar } from "./names.ts"; export const A2UI_VERSION = "v0.9"; export const A2UI_BASIC_CATALOG = "https://a2ui.org/specification/v0_9/catalogs/basic/catalog.json"; @@ -50,15 +51,12 @@ class Ids { get(raw: string): string { const known = this.#map.get(raw); if (known) return known; - let base = - raw === "root" - ? "root" - : raw - .toLowerCase() - .replace(/[^a-z0-9-]+/g, "-") - .replace(/^[^a-z]+/, "") - .replace(/-+$/, "") - .slice(0, 36) || "c"; + // `trimChar`, not `/-+$/`, which backtracks on the "-" runs kept here (js/polynomial-redos). + const dashed = raw + .toLowerCase() + .replace(/[^a-z0-9-]+/g, "-") + .replace(/^[^a-z]+/, ""); + let base = raw === "root" ? "root" : trimChar(dashed, "-").slice(0, 36) || "c"; if (!ID.test(base)) base = `c-${base}`.slice(0, 36); let id = base; for (let n = 2; this.#taken.has(id); n++) id = `${base}-${n}`; diff --git a/packages/core-ts/test/directives.test.ts b/packages/core-ts/test/directives.test.ts index 1a51e6d4..7dc7efa6 100644 --- a/packages/core-ts/test/directives.test.ts +++ b/packages/core-ts/test/directives.test.ts @@ -31,6 +31,14 @@ describe("directive syntax", () => { expect(parseDirective("::embed")).toEqual({ name: "embed", attrs: {}, fence: 2 }); }); + it("trims trailing whitespace in linear time (#168, CodeQL js/polynomial-redos)", () => { + // `/\s+$/` took about 6 s on a 100k run of spaces inside an agent's block. + const start = performance.now(); + expect(parseDirective(`::embed${" ".repeat(200_000)}x`)).toBeUndefined(); + expect(parseDirective("::embed \n\t ")).toEqual({ name: "embed", attrs: {}, fence: 2 }); + expect(performance.now() - start).toBeLessThan(1500); + }); + it("rejects anything that does not parse exactly", () => { for (const bad of [ ":::callout{tone=warning}\nx\n:::", // unquoted value diff --git a/packages/core-ts/test/interop.test.ts b/packages/core-ts/test/interop.test.ts index f3cebdde..c7166d9a 100644 --- a/packages/core-ts/test/interop.test.ts +++ b/packages/core-ts/test/interop.test.ts @@ -527,4 +527,23 @@ describe("json-render", () => { fromJsonRender({ elements: {} }, { sectionId: "sec_x00000001", catalog: catalog.id }), ).toThrow(InteropError); }); + + it("turns an agent's key with a long run of dashes into an id in linear time (#168)", () => { + // The old `/-+$/` trim took about 16 s on this key (the replace before it keeps "-" runs). + const key = `a${"-".repeat(200_000)}b-`; + const start = performance.now(); + const r = fromJsonRender( + { + root: "page", + elements: { + page: { type: "Stack", children: [key] }, + [key]: { type: "Heading", props: { text: "Hi", level: 2 } }, + }, + }, + { sectionId: "sec_jsonrend02", catalog: catalog.id }, + ); + expect(performance.now() - start).toBeLessThan(1500); + expect(r.surface.components[1]?.id).toBe(`a${"-".repeat(35)}`); + expect(checkSurface(r.surface, catalog).issues).toEqual([]); + }); }); diff --git a/packages/core-ts/test/names.test.ts b/packages/core-ts/test/names.test.ts index c5317a43..c786f767 100644 --- a/packages/core-ts/test/names.test.ts +++ b/packages/core-ts/test/names.test.ts @@ -8,6 +8,7 @@ import { resolveTerm, slugify, trimChar, + trimEndChar, } from "../src/names.ts"; const term = ( @@ -59,6 +60,16 @@ describe("foldName", () => { expect(trimChar(`a${"-".repeat(200_000)}b`, "-")).toBe(`a${"-".repeat(200_000)}b`); }); + it("trimEndChar trims one character at the end only, linearly", () => { + expect(trimEndChar("/a/b//", "/")).toBe("/a/b"); + expect(trimEndChar("///", "/")).toBe(""); + expect(trimEndChar("/a", "/")).toBe("/a"); + const inner = `/a${"/".repeat(200_000)}b`; + const start = performance.now(); + expect(trimEndChar(`${inner}/`, "/")).toBe(inner); + expect(performance.now() - start).toBeLessThan(1500); + }); + it("slugify trims dashes at both ends and at the 128-character cut (#168)", () => { expect(slugify(" --Hello, World!-- ")).toBe("hello-world"); expect(slugify("---")).toBe(""); diff --git a/packages/storage/migrations/0030_rate_limits.ts b/packages/storage/migrations/0030_rate_limits.ts index d9377a7b..a1a3faae 100644 --- a/packages/storage/migrations/0030_rate_limits.ts +++ b/packages/storage/migrations/0030_rate_limits.ts @@ -7,9 +7,11 @@ import type { Migration } from "../src/migrate.ts"; * deleted the hourly counters too, which let the reset-mail cap restart every minute. * * `rate_limits`: one row per counter key (`cloudhal:…`), its `count` in the window that began at - * `window_start` (epoch ms). Nothing prunes it by age; a stale row is reset by the next hit. The - * old `cloudhal:` rows in `rateLimit` are deleted (Better Auth would prune them anyway), so a - * counter restarts once at upgrade. + * `window_start` and ends at `expires_at` (epoch ms; each counter has its own window length). + * Keys come from client addresses and submitted email addresses, so `StaffStore.hit` deletes + * expired rows itself, a bounded number at a time, through the `expires_at` index. The old + * `cloudhal:` rows in `rateLimit` are deleted (Better Auth would prune them anyway), so a counter + * restarts once at upgrade. */ export const migration0030RateLimits: Migration = { name: "0030_rate_limits", @@ -17,8 +19,10 @@ export const migration0030RateLimits: Migration = { CREATE TABLE rate_limits ( key TEXT NOT NULL PRIMARY KEY, count INTEGER NOT NULL, - window_start INTEGER NOT NULL + window_start INTEGER NOT NULL, + expires_at INTEGER NOT NULL ); +CREATE INDEX rate_limits_expires ON rate_limits (expires_at); DELETE FROM "rateLimit" WHERE "key" LIKE 'cloudhal:%'; `, }; diff --git a/packages/storage/src/testing.ts b/packages/storage/src/testing.ts index 2ef846d5..84938df8 100644 --- a/packages/storage/src/testing.ts +++ b/packages/storage/src/testing.ts @@ -65,6 +65,17 @@ export function describeSqlDriverConformance(label: string, factory: SqlDriverFa expect(await driver.get("SELECT v FROM t WHERE id = 999")).toBeUndefined(); }); + it("get returns the row of a write's RETURNING clause (an upsert's count in one statement)", async () => { + const upsert = (v: string) => + driver.get<{ n: number }>( + "INSERT INTO t (id, v, n) VALUES (1, ?, 1) ON CONFLICT (id) DO UPDATE SET n = t.n + 1, v = excluded.v RETURNING n", + [v], + ); + expect(await upsert("a")).toEqual({ n: 1 }); + expect(await upsert("b")).toEqual({ n: 2 }); + expect(await driver.get("SELECT v, n FROM t WHERE id = 1")).toEqual({ v: "b", n: 2 }); + }); + it("batch is atomic: a failing statement rolls back the whole batch", async () => { await expect( driver.batch([ diff --git a/scripts/size-baseline.json b/scripts/size-baseline.json index 8d97ee22..db95a8a3 100644 --- a/scripts/size-baseline.json +++ b/scripts/size-baseline.json @@ -22,7 +22,7 @@ "packages/core-ts/src/menu.ts": 928, "packages/core-ts/src/releases.ts": 996, "packages/core-ts/src/store.ts": 1212, - "packages/core-ts/src/surface-interop.ts": 1235, + "packages/core-ts/src/surface-interop.ts": 1233, "packages/core-ts/src/taxonomy.ts": 1718, "packages/mcp-server/src/menu-tools.ts": 1058, "packages/mcp-server/src/schedule-tools.ts": 1596,