Skip to content

fix(core): REST + CLI audit, 34 bug fixes#820

Merged
ascorbic merged 3 commits into
mainfrom
fix/rest-cli-audit
Apr 29, 2026
Merged

fix(core): REST + CLI audit, 34 bug fixes#820
ascorbic merged 3 commits into
mainfrom
fix/rest-cli-audit

Conversation

@ascorbic
Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes 34 bugs across the REST API and CLI discovered during a systematic audit that exercised every API surface against the deployed beta site and then traced each failure through source. Mirrors the approach from PR #777 (MCP audit) but covers the REST and CLI surfaces.

The audit ran 6 parallel adversarial review agents across content routes, auth/scope middleware, pagination, schema/sections/settings, media/search/redirects/comments, and CLI parity. The fixes went through 2 adversarial review passes before finalizing.

Security fixes (6)

Bug Route Fix
Subscriber sees draft data on published content (CRITICAL) GET /content/:col/:id Strip draft hydration for users without content:read_drafts
Revision GET leaks draft data to subscribers GET /revisions/:id Require content:read_drafts instead of content:read
Search exposes drafts to anonymous users GET /search?status=draft Force status=published for users without content:read_drafts
Scope middleware rejects valid taxonomies:manage/menus:manage tokens Middleware Use granular scopes in SCOPE_RULES
Scope middleware rejects valid settings:read/settings:manage tokens Middleware Split settings rule into GET/WRITE with granular scopes
Media confirm lacks ownership check POST /media/:id/confirm Add requireOwnerPerm check

Error handling fixes (4)

Bug Route Fix
Publish missing content returns 500 POST /content/:col/:id/publish Catch EmDashValidationError, return VALIDATION_ERROR
Unpublish missing content returns 500 POST /content/:col/:id/unpublish Same pattern
Unschedule missing content returns 500 DELETE /content/:col/:id/schedule Same pattern
CLI whoami refresh corrupts stored credentials CLI whoami Unwrap { data: ... } envelope from token refresh response

Data integrity fixes (4)

Bug Route Fix
Revision restore not transactional Handler Wrap in withTransaction
Content-terms stores slug as entry_id POST /content/:col/:slug/terms/:tax Resolve canonical ID from handler result
Auth code exchange token creation not transactional Handler Wrap both INSERTs in withTransaction
Cache invalidation uses slug not ID in restore/discard-draft Routes Extract resolvedId from existing item

Client + CLI fixes (7)

Bug Fix
client.taxonomies() returns undefined Unwrap { taxonomies } not { items }
client.menus() returns undefined Return bare array, not .items
refreshInterceptor parses wrong envelope Handle both { data: { access_token } } and bare shapes
CLI draft overlay bypasses PT-to-markdown conversion Run convertDataForRead on overlay data
CLI void commands (delete, publish, etc.) empty JSON stdout Output { success: true } in --json mode
Plugin builtin detection misses static ESM + subpath imports Rewrite regex for import X from, export { } from, node:fs/promises
Auth code exchange doesn't revalidate user role Re-fetch role and re-clamp scopes before token issuance

Defense in depth (12)

Bug Fix
Sections routes crash on uninitialized runtime Add requireDb guards
Redirects routes crash on uninitialized runtime Add requireDb guards
Section create allows source: "theme" (undeletable) Restrict schema to "user" | "import"
Snapshot inaccessible to session-authenticated users Add manual session resolution in route
Device flow ignores client_id Validate against _emdash_oauth_clients
Revision limit accepts negative values (LIMIT -1 = unlimited) Clamp with Math.max(1, ...)
Media provider upload has no size/type validation Add basic validation
Schema cursor fields missing max-length Add .max(2048) to sections, comments, users schemas
Redirect search doesn't escape LIKE wildcards Escape %/_/\ with ESCAPE '\\' clause
PKCE challenge not constant-time Use secureCompare
OpenAPI let singleton violates globalThis convention Move to Symbol.for pattern
Credential directory default permissions Add mode: 0o700

Performance (1)

Bug Fix
Taxonomy terms N+1 count queries Batch countEntriesForTerms with chunking at SQL_BATCH_SIZE

Intentional breaking changes

All three are security fixes where the old behavior was the bug:

  1. Revision GET now requires content:read_drafts (was content:read). Subscribers can no longer fetch individual revisions.
  2. Search ?status=draft forced to published for anonymous/subscriber users. Draft search now requires content:read_drafts.
  3. Section create rejects source: "theme". Only "user" and "import" are accepted via API.

Skipped (3)

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes (pre-existing @oslojs import errors excluded)
  • pnpm lint passes (0 diagnostics)
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (N/A, no admin UI string changes)
  • I have added a changeset (will add after review)
  • New features link to an approved Discussion (N/A)

AI-generated code disclosure

  • This PR includes AI-generated code

Driven by Claude (Opus 4.6) with systematic audit against the deployed beta site, 6 parallel adversarial review agents for bug discovery, and 2 adversarial review passes on the final diff.

Test output

Test Files  6 passed (6)     # directly affected test files
     Tests  127 passed (127)

Full unit suite: 2043 passing, 0 new failures (4 pre-existing failures from @oslojs import in secrets module).

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 29, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-perf-coordinator 3c5bc24 Apr 29 2026, 02:08 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 29, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-i18n 3c5bc24 Apr 29 2026, 02:09 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 29, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs 3c5bc24 Apr 29 2026, 02:09 PM

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 29, 2026

⚠️ No Changeset found

Latest commit: 3c5bc24

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

Scope check

This PR changes 734 lines across 38 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.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 29, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-cache 3c5bc24 Apr 29 2026, 02:09 PM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 29, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-playground 3c5bc24 Apr 29 2026, 02:10 PM

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 29, 2026

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@820

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@820

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@820

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@820

emdash

npm i https://pkg.pr.new/emdash@820

create-emdash

npm i https://pkg.pr.new/create-emdash@820

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@820

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@820

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@820

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@820

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@820

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@820

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@820

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@820

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@820

commit: 3c5bc24

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Systematic bug-fix sweep across packages/core REST API + client SDK + CLI to align response envelopes, harden auth/scope enforcement, fix data integrity edge cases, and add targeted defense-in-depth/perf improvements discovered during an audit.

Changes:

  • Tighten authorization and data-leak protections (draft/revision/search restrictions, media confirm ownership, scope-rule fixes, DB guards).
  • Fix client/CLI parity issues (response envelope handling, refresh token parsing, CLI JSON output consistency, PT conversion on draft overlays).
  • Improve robustness/perf (transactional revision restore + auth-code token issuance, redirect LIKE escaping, batched taxonomy term counts, pagination/length clamps).

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/core/tests/unit/client/transport.test.ts Adds unit coverage for refreshInterceptor handling of { data: ... } token envelopes.
packages/core/tests/unit/client/client.test.ts Adds regression tests for /taxonomies and /menus response envelope shapes.
packages/core/tests/integration/cli/cli.test.ts Adds CLI integration coverage for taxonomy/menu JSON output.
packages/core/src/database/repositories/taxonomy.ts Adds batched term entry-count query to remove N+1 counting.
packages/core/src/database/repositories/redirect.ts Escapes LIKE wildcards and adds explicit ESCAPE handling for redirect/404 search.
packages/core/src/client/transport.ts Makes refresh-token parsing tolerant of wrapped and bare JSON shapes.
packages/core/src/client/index.ts Fixes SDK methods to match actual API response shapes for taxonomies/menus.
packages/core/src/cli/credentials.ts Ensures credential directory is created with restrictive permissions.
packages/core/src/cli/commands/login.ts Fixes CLI whoami refresh parsing for { data: ... } responses.
packages/core/src/cli/commands/content.ts Applies PT-to-markdown conversion to draft overlays; emits { success: true } for void JSON commands.
packages/core/src/cli/commands/bundle-utils.ts Expands Node builtin import detection for more ESM/export forms.
packages/core/src/astro/routes/api/snapshot.ts Restores session-auth access path for snapshot route while keeping preview-signature auth.
packages/core/src/astro/routes/api/sections/index.ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/sections/[slug].ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/search/index.ts Forces non-draft readers to status=published to prevent draft search leakage.
packages/core/src/astro/routes/api/revisions/[revisionId]/index.ts Requires content:read_drafts for revision reads (security hardening).
packages/core/src/astro/routes/api/redirects/index.ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/redirects/[id].ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/redirects/404s/summary.ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/redirects/404s/index.ts Adds requireDb guard to avoid crashes on uninitialized runtime.
packages/core/src/astro/routes/api/openapi.json.ts Moves OpenAPI caching to globalThis/Symbol.for and hardens error handling/cache headers.
packages/core/src/astro/routes/api/media/providers/[providerId]/index.ts Clamps provider list limit; adds basic upload size/type checks.
packages/core/src/astro/routes/api/media/[id]/confirm.ts Adds ownership-based authorization to confirm/fail pending uploads.
packages/core/src/astro/routes/api/content/[collection]/[id]/terms/[taxonomy].ts Ensures term assignment uses canonical content ID (slug→id correctness).
packages/core/src/astro/routes/api/content/[collection]/[id]/revisions.ts Clamps revision list limit to avoid negative/unbounded behavior.
packages/core/src/astro/routes/api/content/[collection]/[id]/restore.ts Uses resolved canonical ID for restore + cache invalidation tags.
packages/core/src/astro/routes/api/content/[collection]/[id]/discard-draft.ts Uses resolved canonical ID for discard-draft + cache invalidation tags.
packages/core/src/astro/routes/api/content/[collection]/[id].ts Strips draft overlay fields for users without content:read_drafts.
packages/core/src/astro/middleware/auth.ts Updates scope rules for granular taxonomies:manage/menus:manage and settings read/manage.
packages/core/src/api/schemas/users.ts Adds max-length clamp for cursor parameters.
packages/core/src/api/schemas/sections.ts Tightens section creation source values and clamps list cursor/limit defaults.
packages/core/src/api/schemas/comments.ts Clamps list cursor/limit defaults for comments.
packages/core/src/api/handlers/taxonomies.ts Switches term counts to use new batched repository method.
packages/core/src/api/handlers/revision.ts Makes revision restore transactional (best-effort on D1).
packages/core/src/api/handlers/oauth-authorization.ts Uses constant-time PKCE compare; revalidates role/scopes; stores tokens transactionally.
packages/core/src/api/handlers/device-flow.ts Adds commentary around client_id behavior in device code request.
packages/core/src/api/handlers/content.ts Maps EmDashValidationError to VALIDATION_ERROR for publish/unpublish/unschedule paths.
packages/blocks/src/validation.ts Tightens numeric type checks for block validation bounds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/astro/routes/api/media/providers/[providerId]/index.ts Outdated
Comment thread packages/core/src/api/handlers/device-flow.ts
Comment thread packages/core/src/cli/commands/bundle-utils.ts
@github-actions
Copy link
Copy Markdown
Contributor

Overlapping PRs

This PR modifies files that are also changed by other open PRs:

This may cause merge conflicts or duplicated work. A maintainer will coordinate.

@ascorbic ascorbic merged commit d465111 into main Apr 29, 2026
33 checks passed
@ascorbic ascorbic deleted the fix/rest-cli-audit branch April 29, 2026 14:23
ascorbic added a commit that referenced this pull request Apr 29, 2026
The Apply workflow has been silently broken for ~2 days, so PRs that
landed in that window (#820, #816, #811) merged with stale snapshots
in the repo. CI's Measure on this PR shows the real per-route counts
on top of current main: uniform +1 cold and +1 warm on all public
routes except /pages/about and /rss.xml. This is pre-existing drift,
not caused by this PR — it's the natural consequence of the Apply
workflow failing to commit the auto-updated snapshots that the
Measure step correctly produced on those merged PRs.

Apply CI's regenerated snapshots from this PR's Measure run so main
ends up in a consistent state once this lands. Future PRs will then
get their own real drift signal rather than inheriting this baseline
correction.
ascorbic added a commit that referenced this pull request Apr 29, 2026
* fix(ci): query-counts-apply workflow no longer silently fails

The Apply workflow has been silently broken for ~2 days, never pushing
auto-updated snapshots to PRs that drifted. Two distinct bugs:

(1) Same-repo PRs: the artifact was unpacked into
    $GITHUB_WORKSPACE/artifact, but the later actions/checkout step
    defaults to clean: true and wipes the workspace, deleting our
    payload before the Apply step's cp could read it. Stage under
    $RUNNER_TEMP/qc-artifact instead, which sits outside the workspace.

(2) Fork PRs: the cross-check used listPullRequestsAssociatedWithCommit
    against the base repo, which doesn't reliably surface fork PRs from
    that view, so valid fork artifacts were rejected as 'not associated
    with commit'. Replace with pulls.get(prNumber) and verify
    pr.head.sha === workflow_run.head_sha — equivalent tamper-resistance
    (a forged pr-number won't match), works for forks.

Add pull-requests: read so pulls.get works when the default token
permissions are restricted.

* ci: reconcile query-count snapshot baseline

The Apply workflow has been silently broken for ~2 days, so PRs that
landed in that window (#820, #816, #811) merged with stale snapshots
in the repo. CI's Measure on this PR shows the real per-route counts
on top of current main: uniform +1 cold and +1 warm on all public
routes except /pages/about and /rss.xml. This is pre-existing drift,
not caused by this PR — it's the natural consequence of the Apply
workflow failing to commit the auto-updated snapshots that the
Measure step correctly produced on those merged PRs.

Apply CI's regenerated snapshots from this PR's Measure run so main
ends up in a consistent state once this lands. Future PRs will then
get their own real drift signal rather than inheriting this baseline
correction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants