Review follow-ups: MCP OAuth step-up scope set (phase 2) and CI/docs housekeeping #168
Description
Small follow-ups from the adversarial review of 0.6.3; all latent or housekeeping.
MCP OAuth step-up (fold into RFC-0044 phase 2, #144; latent until a runtime sets grantId)
packages/api/src/mcp-oauth.ts:177-182: step-up recomputes the missing scope set fromtool.scopesand testsholds(role, s)literally, ignoring the own-variant lattice andproposeScopesthe policy engine uses, so it never fires for Authors/Contributors and over-asks for propose calls; it also judges the installation role where the handler judges the site role (RFC-0034 A2). Have the policy engine return the missing set and reuse it.packages/api/src/mcp-mount.ts:62: the step-up 403 skips the audit log; a token principal's scope denial should leave a row.packages/api/src/playground.ts:63: the deny rows for the two well-known routes are unenforceable (isBlockedInPlaygroundreturnsundefinedfor non-/api/paths except/mcp); either enforce them or drop the rows and the route-table test.- RFC-0044 line ~580 says existing deployments get the challenge after phase 1; in 0.6.3 nothing sets
mcp.oauth, so say "once a runtime sets it (phase 2)". Record in RFC-0044 that the protected-resource document is served without CORS (consistent with/mcprefusing cross-origin) and assert atcreateApitime thatresourceends in/mcp.
CI and docs housekeeping (one ci: PR)
.github/workflows/codeql.yml:26:cancel-in-progresskeyed on the ref cancels a running default-branch or scheduled analysis on every push tomain; scope cancellation to pull requests.docs/adr/0012-cloudflare-deployment.md:53: the addendum still says the undici override waits;pnpm-workspace.yamlhas carried it since fix(deps): close the open Dependabot security alerts #159.- test: quiet the known log noise #161's Studio fix (
SiteStatusCard.tsxBadge variant) shipped in 0.6.3 without a changeset; add a line to the 0.6.3 notes or the next changeset.
Source: adversarial review run after the 0.6.3 release (178 agents, 47 confirmed findings).
Activity
Madraka commentedon Nov 8, 2026
One more item from the pre-merge review of #169. It already applies on Workers, and #169 brings it to Node.
Better Auth's rate-limit pruning can wipe Cloudhal's long-window counters. Cloudhal keeps its 1-hour per-member reset-mail cap (3 per hour) in Better Auth's
rateLimittable (seepackages/api/src/auth/instance.ts). When a window expires, Better Auth deletes everyrateLimitrow older than its own 60 s window, and that includes Cloudhal's hourly counters.Scenario: an attacker requests resets for one address until the hourly cap is reached. They wait about 61 s and send any auth request from the same IP, which triggers the prune. They can then request three more resets. The effective cap becomes about 3 per minute. The per-IP limit of 10 per minute still holds.
Fix: keep Cloudhal's own counters in a separate table, or store them in a form the prune cannot reach. Add a test that lets the 60 s window lapse between reset bursts.
Madraka commentedon Nov 8, 2026
Two more items from the #169 review.
packages/core-ts/src/names.ts:66(slugify, used by export) andpackages/core-ts/src/media/types.ts:257(slugifyFilename) still use.replace(/^-+|-+$/g, ""). CodeQL flagged this pattern inpackages/cli/src/commands/init.ts, and fix(cli,api): self-host auth hardening (#165) #169 replaced it there with a linear loop. Do the same here, so CodeQL does not raise it again when these files next change.slugifywould also pass against the old regex code: the first replace already collapses runs. Only thedescribeSetupcase actually tells old code from new. The fix is fine, the test is not a regression guard. When you do item 9, add one input that the old two-pass version handled badly, or drop the timing claim.fix: review round 2 for #168 (rate-limit pruning, step-up loops, ReDoS)
Merge pull request #173 from cloudhal-dev/claude/suspicious-satoshi-9…
Madraka commentedon Nov 8, 2026
Fixed by #173 (all ten items, plus the review round: rate_limits pruning, single-statement RETURNING count, plan-mode/skill denials get no step-up, challenge capped by both site and installation role, surface-interop linear trim). The second ReDoS sweep the verification turned up is #174.
test: align ReDoS timing bounds; code-point cap in ancestorPaths (#174)