Registry: icon, screenshot, banner artifacts end-to-end#1239
Merged
Conversation
Wire release.artifacts.{icon,screenshot,banner} through publish and admin.
CLI:
- Manifest schema gains a `release.artifacts` block: icon/banner as single
`{ file }` refs, screenshot as an array. JSON Schema regenerated.
- `publish` resolves each ref relative to the manifest, measures dimensions
and content type via image-size, uploads to `--artifact-base-url`, and
embeds `{ url, checksum, contentType, width, height, lang? }` in the
release. The lexicon types `screenshot` as a single artifact, so the first
screenshot uses that slot and extras ride in `x-screenshot-N` custom keys.
- image-size catalog-pinned.
Server:
- New admin proxy `GET /registry/artifact?url=` for publisher-supplied image
URLs. Applies SSRF defences via assertSafeArtifactUrl (re-validating each
redirect hop), enforces an image content-type allowlist, caps the body,
and serves back with `private, no-store` plus attachment + sandbox CSP so
a navigated SVG can't execute in the admin origin.
Admin:
- RegistryPluginDetail renders the icon, banner, and a screenshot gallery
through the proxy. Every image URL goes through artifactProxyUrl (scheme
allow-list) before the proxy.
Prefix each uploaded artifact URL with its role/index slot (icon-, banner-, screenshot-N-) so two refs that share a basename in different source directories no longer collapse to one upload target. Previously two screenshots named shot.png in light/ and dark/ both mapped to <base>/<slug>/<version>/shot.png; the second PUT overwrote the first and both records pointed at the same URL. Also tighten the publish-side path-escape guard to reject the `..` segment precisely (plus absolute paths) instead of any relative path beginning with two dots, which false-positived filenames like `..config.png`. Drop image/avif from the proxy allowlist so the served content types match what the CLI can actually produce. Add tests: same-basename screenshots/icon get distinct URLs, a two-dot filename is accepted, and a streamed body with no content-length that exceeds the cap is rejected with 413 via readCapped.
The #1033 implementation stored extra screenshots under x-screenshot-N custom artifact keys, which is not FAIR-aligned. FAIR's artifacts map allows an artifact value to be a list of objects; model screenshots as a first-class array end to end. - lexicon: artifacts.screenshot (single ref) -> screenshots (array of #artifact, maxLength 8); regenerate atcute types - plugin-cli manifest schema: release.artifacts.screenshot -> screenshots; regenerate JSON schema - publish path: write artifacts.screenshots as an array, drop the x-screenshot-N spillover; keep the collision-free slot-prefixed upload URL scheme (screenshot-N-<filename>) - admin: read the screenshots array directly, drop x-screenshot-N collection and numeric ordering
…shots array cap The bundle path's MAX_SCREENSHOTS governed a separate ingestion route from the lexicon/manifest screenshots array (capped at 8). Align them so all screenshot caps agree.
🦋 Changeset detectedLatest commit: 18f1b21 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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 | 18f1b21 | May 31 2026, 01:53 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 18f1b21 | May 31 2026, 01:55 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 18f1b21 | May 31 2026, 01:54 PM |
github-actions
Bot
added
review/needs-review
Contributor
Scope checkThis PR changes 1,830 lines across 25 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. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@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-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for plugin registry image artifacts (icon, banner, screenshot gallery): manifest/schema + CLI upload/recording + server-side SSRF-defended proxy + admin UI rendering.
Changes:
- Extend the registry lexicon from singular
screenshotto an orderedscreenshots[]gallery (capped at 8). - Add plugin CLI support to validate/measure image files (via
image-size), upload them to a publisher-provided base URL, and embed artifact records into the release. - Add an admin-side artifact proxy route with SSRF defenses + image allowlist, and render icon/banner/gallery on the registry plugin detail page.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds image-size to the workspace catalog. |
| pnpm-lock.yaml | Locks image-size and updates dependency snapshots. |
| packages/registry-lexicons/src/generated/types/com/emdashcms/experimental/package/release.ts | Updates generated type schema to use screenshots array with max length. |
| packages/registry-lexicons/lexicons/com/emdashcms/experimental/package/release.json | Updates lexicon definition from screenshot to screenshots[] (max 8) and adjusts description. |
| packages/plugin-cli/tests/publish.test.ts | Adds release-record writing tests for icon/banner/screenshots. |
| packages/plugin-cli/tests/publish-upload-artifacts.test.ts | Adds tests for resolving, uploading, and error handling for artifact files. |
| packages/plugin-cli/tests/publish-artifacts.test.ts | Adds tests for image sniffing/dimension extraction + record building/checksum. |
| packages/plugin-cli/tests/manifest-schema.test.ts | Adds schema tests for release.artifacts and rejects legacy screenshot. |
| packages/plugin-cli/src/publish/upload-artifacts.ts | Implements artifact path resolution, image measurement, upload, and record construction. |
| packages/plugin-cli/src/publish/artifacts.ts | Implements content-type allowlist + dimension extraction + checksum generation for images. |
| packages/plugin-cli/src/publish/api.ts | Extends publish API types and applies resolved artifacts into release records. |
| packages/plugin-cli/src/manifest/translate.ts | Plumbs release.artifacts through manifest normalization. |
| packages/plugin-cli/src/manifest/schema.ts | Adds Zod schemas/types for release.artifacts (icon/banner/screenshots). |
| packages/plugin-cli/src/commands/publish.ts | Adds --artifact-base-url and resolves/uploads artifacts during publish. |
| packages/plugin-cli/src/bundle/utils.ts | Raises screenshot cap to 8 in the plugin-cli bundle utilities. |
| packages/plugin-cli/schemas/emdash-plugin.schema.json | Updates generated JSON Schema to include release.artifacts. |
| packages/plugin-cli/package.json | Switches image-size to use the workspace catalog version. |
| packages/core/tests/unit/api/registry-artifact-proxy.test.ts | Adds unit tests covering auth, SSRF defenses, content-type allowlist, and size caps for the proxy. |
| packages/core/src/astro/routes/api/admin/plugins/registry/artifact.ts | Adds SSRF-defended, image-only proxy endpoint for registry artifact URLs. |
| packages/core/src/astro/integration/routes.ts | Registers the new admin proxy route in core route injection. |
| packages/core/src/api/handlers/index.ts | Re-exports assertSafeArtifactUrl for route use. |
| packages/admin/tests/lib/registry-artifacts.test.ts | Adds tests for proxy URL construction and artifact extraction/narrowing. |
| packages/admin/src/lib/api/registry.ts | Adds artifactProxyUrl() and extractMediaArtifacts() utilities. |
| packages/admin/src/components/RegistryPluginDetail.tsx | Renders icon/banner/screenshot gallery via the server proxy. |
| .changeset/registry-image-artifacts.md | Publishes a changeset describing the new feature across CLI/core/admin. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+121
to
+125
| "artifact-base-url": { | ||
| type: "string", | ||
| description: | ||
| "Base URL the CLI PUTs media artifacts (icon / screenshot / banner) to. Each file is uploaded to <base>/<slug>/<version>/<filename> and that URL is recorded in the release. The target must serve the bytes back unchanged with a stable content type. Required when the manifest declares any `release.artifacts`.", | ||
| }, |
Comment on lines
+78
to
+92
| response = await fetch(current.href, { redirect: "manual", signal: controller.signal }); | ||
| if (response.status < 300 || response.status >= 400) break; | ||
| const location = response.headers.get("location"); | ||
| if (!location) break; | ||
| if (hop === MAX_REDIRECTS) { | ||
| return apiError("ARTIFACT_URL_REJECTED", "Too many redirects", 502); | ||
| } | ||
| let next: URL; | ||
| try { | ||
| next = await assertSafeArtifactUrl(new URL(location, current).href); | ||
| } catch { | ||
| return apiError("ARTIFACT_URL_REJECTED", "Redirect target is not allowed", 400); | ||
| } | ||
| current = next; | ||
| } |
Comment on lines
24
to
+28
| export const MAX_BUNDLE_SIZE = 256 * 1024; | ||
| export const MAX_FILE_SIZE = 128 * 1024; | ||
| export const MAX_FILE_COUNT = 20; | ||
|
|
||
| export const MAX_SCREENSHOTS = 5; | ||
| export const MAX_SCREENSHOTS = 8; |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is a well-scoped, end-to-end feature adding icon / screenshot / banner artifacts to the experimental plugin registry. The approach is sound: manifest file refs → CLI resolution/upload with image-size → lexicon-aligned release records → SSRF-defended server proxy → Lingui-wrapped admin render.
What I checked
- Architecture & approach: The trust boundary is correctly drawn at the server-side proxy (
artifact.ts) with DNS-rebinding defenses, redirect re-validation, content-type allowlist, byte caps, andprivate, no-storecache headers. SVG active-content risks are mitigated withContent-Disposition: attachment+ sandbox CSP. The CLI path traversal guard (resolveWithinManifest) prevents..escapes. Cap counts line up across the lexicon (maxLength: 8), manifest schema (z.array(...).max(8)), bundle utils (MAX_SCREENSHOTS = 8), and JSON schema (maxItems: 8). - AGENTS.md conventions: All new admin strings are wrapped via Lingui
tmacros. No physical Tailwind directional classes (ml-/mr-/etc.) appear in new UI code. The route declaresprerender = falseand uses the standardrequirePerm/apiErrorhelpers. A changeset is present. - Security:
assertSafeArtifactUrlis invoked before every fetch and re-invoked on every redirect hop. The proxy rejects non-image content types and oversized bodies. The admin never renders a publisher URL directly in<img src>— it always routes through the proxy. - Tests: New unit tests cover the proxy route, artifact measurement, upload resolution, manifest schema rules, and publish record construction. Existing publish tests are extended for artifact fields.
- Data integrity:
buildArtifactRecordcomputes the same multihash checksum the consumer will verify. Empty / malformed artifact entries are dropped byasMediaArtifact, so a bad aggregator record can't crash the detail view.
Headline conclusion: I found no logic bugs, security gaps, convention violations, or missing tests. The implementation is clean and ready to merge.
github-actions
Bot
added
review/approved
Contributor
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. |
Merged
18 tasks
…rd; forbid SVG, allow AVIF The artifact proxy no longer accepts a caller-supplied URL. The client addresses an artifact by coordinates (did, slug, version, kind, index); the server resolves the declared URL from the validated release record, so the proxy can only ever fetch a publisher-declared artifact. SSRF defences remain as a second layer on the resolved URL (incl. every redirect hop). SVG is dropped from both the proxy allowlist and the publish CLI (active content); AVIF is added end-to-end.
github-actions
Bot
added
review/needs-rereview
Merged
ascorbic
added a commit
that referenced
this pull request
May 31, 2026
Resolves overlap with the merged #1239 (artifacts): the manifest keeps both release-level `requires` (flat) and the `release.artifacts` block; the publish path writes both; handler barrel exports both registry helpers; manifest-schema tests keep both describe blocks. JSON schema regenerated.
ascorbic
added a commit
that referenced
this pull request
May 31, 2026
Aligns env constraints with release.artifacts (merged in #1239) under a single release block, matching the RFC. repo stays top-level (documented legacy for backwards compatibility). Publish reads manifest.release.requires; the normalised manifest keeps a flat requires field.
ascorbic
added a commit
that referenced
this pull request
May 31, 2026
…in compatibility warning (#1238) * feat(registry): release env requires + admin compatibility gate Plugins published to the experimental registry can declare release-level environment constraints. A manifest's `requires` block (e.g. `{ "env:emdash": ">=1.0.0", "env:astro": ">=4.16" }`) is validated at publish time and written into the release record. - registry-client: new dependency-free `./env` module shared by the CLI, server, and admin — `parseRequires` (guards the lexicon-`unknown` value), `isValidVersionRange`, `satisfiesRange`, and `checkEnvCompatibility` over a focused semver-range grammar (comparators, caret, tilde, partial versions, wildcard, AND sets). - plugin-cli: `RequiresSchema` on the manifest, threaded release-level into `publishRelease` (never via the profile input); JSON Schema regenerated. - core: capture the host Astro version in `astro:config:setup` and surface it (with EmDash VERSION) on the admin manifest. New `assertEnvCompatible` gate refuses incompatible install AND update with `ENV_INCOMPATIBLE` (409), placed after yank-check, before the artifact fetch. - admin: RegistryPluginDetail reads host versions, renders a localized compatibility warning and disables Install when unsatisfied. Closes #1031. * fix(registry): apply adversarial review findings for env requires Share the version->env:* host map between server and admin via a single hostEnvFromVersions helper, dropping the admin's redundant /manifest fetch and the duplicated dev-skip rule (admin now derives host env from the manifest query the shell already runs). Log skipped env constraints server-side (findSkippedEnvConstraints) so a host version the gate can't evaluate is observable rather than a silent bypass. Cover the gate end-to-end through handleRegistryUpdate with a mocked DiscoveryClient, asserting ENV_INCOMPATIBLE aborts before any artifact fetch. Document the accepted, more-permissive prerelease range semantics relative to node-semver. * refactor(registry): use node-semver for env requires range evaluation Replace the hand-rolled range evaluator in registry-client's env module with the semver package. Adds || union support and node-semver prerelease gating; satisfiesRange passes includePrerelease so a prerelease host build is evaluated by precedence rather than excluded from release-only ranges (a prerelease host is not a definite mismatch). isValidVersionRange (shared by the publish-time RequiresSchema) and the fail-open gate semantics are unchanged. * refactor(registry): drop redundant buildHostEnv wrapper buildHostEnv was a pure pass-through to hostEnvFromVersions (the shared registry-client/env helper). The install/update routes now call hostEnvFromVersions directly, and its duplicate dev-skip/astro-omit test block is removed (covered by registry-client's env tests). * refactor(registry): nest requires under release.requires Aligns env constraints with release.artifacts (merged in #1239) under a single release block, matching the RFC. repo stays top-level (documented legacy for backwards compatibility). Publish reads manifest.release.requires; the normalised manifest keeps a flat requires field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds icon / screenshot / banner artifacts to the experimental plugin registry, end to end. Plugin authors declare images in
emdash-plugin.jsoncunderrelease.artifacts(icon/banneras single refs,screenshotsas an ordered array);emdash-plugin publish --artifact-base-url <url>resolves each ref relative to the manifest, measures dimensions + content type viaimage-size, uploads the bytes, and embeds{ url, checksum, contentType, width, height, lang? }in the release record. A new admin proxy route fetches the arbitrary publisher-supplied image URLs with full SSRF defences and an image content-type allowlist, and the plugin detail page renders the icon, banner, and a screenshot gallery through it.Screenshots are a first-class
screenshotsarray (a list of artifact objects), matching FAIR's WordPress/TYPO3 extension shape; FAIR's singularscreenshotalias is a transport-boundary concern and is not carried on the record. Screenshot count caps agree across the lexicon, manifest schema, and bundle path.Part of the registry lexicon-to-UI umbrella (#1026).
Closes #1033.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges included.AI-generated code disclosure
Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
feat/registry-artifacts. Updated automatically when the playground redeploys.