Repository navigation
feat(plugins): add safe Block Kit navigation and locale context - #3174
Conversation
🦋 Changeset detectedLatest commit: d3e18f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | d3e18f0 | Sep 20 2026, 02:56 AM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Scope checkThis PR changes 2,050 lines across 53 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
/review |
|
OK enough. This might return 404 if no schema. Not critical. Now let me also look at whether the runtime properly reject an admin route request from an unauthenticated user (CSRF is only one part): Actually the auth middleware handles that. Not a Block Kit concern. Given the exhaustive prior review, I only find MINOR issues not raised before:
None of these are HIGH/CRITICAL. Given the exhaustiveness of the prior review and the maintainer's active engagement fixing issues, this PR looks solid. Let me check for one more potential real bug in the runtime validation. If the plugin is enabled, and it's in Given my analysis, the PR is in good shape. Let me confirm one final thing — check if any test failures could be hidden. Actually I don't need to run tests since the PR description confirms all pass. My verdict: The prior review was thorough and the fixes are correct. My additional findings are LOW severity nits about edge cases that don't materially affect security or correctness. I don't think this warrants another round of findings. However, the instructions say: "If the PR looks good, respond with only 'LGTM!' and skip posting a review." Given:
I'll respond with LGTM. LGTM! |
72e5a9a to
0a34261
Compare
0a34261 to
d3e18f0
Compare
There was a problem hiding this comment.
I reviewed the full diff for PR #3174 and traced the cross-cutting changes across @emdash-cms/blocks, @emdash-cms/admin, emdash, @emdash-cms/cloudflare, @emdash-cms/plugin-test, and the plugin-cli skill docs.
What I checked
- New
LinkElement/NavigationElementtype and rendering in@emdash-cms/blocks. resolvePluginLinkTargetand safe link URL construction in the admin SPA.- Server-side validation of sandboxed admin/widget responses (
validateBlockResponse,validateSandboxedAdminResponse, bounds, image-host policy, plugin-page path policy, link-in-form prevention). - UI context attestation: how locale/direction/surface is resolved from the request/cookie and propagated through native, Cloudflare, workerd, and plugin-test runtimes via
routeCtx.ui. - Race/request cancellation via
requestGenerationin the sandboxed admin components. - New
admin.loadPage/loadWidget/act/submithelpers inplugin-test. - Changeset quality against
.changeset/README.md.
Headline conclusion
The implementation matches the PR description and EmDash conventions. The validation boundary is fail-closed, the link construction is safely encoded, and the runtime plumbing for host-attested locale context is consistent across all three isolate backends. The changeset is clear, correctly scoped, and appropriately marked as a minor security tightening with migration guidance.
I found no blocking logic bugs, security regressions, or contract breaks. The PR is good to ship.
…and widgets (#3322) Block Kit pages and dashboard widgets of plugins registered in `plugins: []` received no `routeCtx.ui`: the UI context is resolved from getSandboxedAdminDefinition(), which only knows sandboxed entries and the marketplace cache. A standard-format plugin therefore got the administrator's locale and direction when installed from the registry and undefined when the same code was registered in the site config, so it could not localize there. Configured plugins now get the same context from the pages and widgets they declare. Two differences from the sandboxed path are deliberate: a request for an undeclared surface is served without context instead of rejected with 400, because configured plugins were never held to their declarations; and their responses are still not validated against the sandbox policy, as decided in the review of #3174.
A native plugin with admin.pages or admin.widgets and no admin.entry renders Block Kit, but the Block Kit page only showed the sandboxed form. Adds the native recipe (explicit admin route, ctx.input) and states that ctx.ui and response validation from #3174 apply only to sandboxed pages and widgets. Refs #2982 Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What does this PR do?
Adds safe, structured navigation and host-attested locale context to sandboxed Block Kit pages and dashboard widgets.
linkelements for saved content, declared plugin pages, generated plugin settings, and external HTTP, HTTPS, ormailto:destinations. The host constructs internal admin URLs; external links open withnoopener noreferrerand never dispatch block actions.routeCtx.ui.admin.loadPage(),loadWidget(),act(), andsubmit()plugin-test helpers, plus public docs and canonical/generated authoring-skill guidance.This intentionally fails closed for existing sandboxed plugins that return external Block Kit images without matching network authority. The changeset includes the migration: declare
network:requestplus the image hosts, ornetwork:request:unrestricted, publish an update, and obtain renewed administrator consent. Root-relative images remain available without network authority.Related feature discussion: #3004. This maintainer-directed lower-stack prerequisite does not add editor panels, actions, or unsaved content transport.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
The first screenshot shows Kumo-rendered primary, secondary, and inline external link appearances in English. The second uses Arabic text and right-to-left direction on the same plugin surface. The rendered external link includes the external indicator and
noopener noreferrer; keyboard verification focused the structured content link with its host-resolved URL.Validation completed at
166825740adc5d89ff969dce177610352ac5797b:pnpm install --frozen-lockfilepnpm buildpnpm typecheckpnpm lintpnpm lint:quickpnpm lint:json | jq '.diagnostics | length'(0)pnpm format:check@emdash-cms/blockstestspnpm --dir docs build(85 pages)The required independent adversarial review initially found capability-consent, compatibility, host-resource-bound, and stale-response issues. Those findings were fixed and re-reviewed at the current head; the final review reports no actionable findings.