Skip to content

fix: surface superstruct validation errors and normalize update-API paths#1546

Open
drlukeangel wants to merge 2 commits into
Thinkmill:mainfrom
drlukeangel:surface-superstruct-errors-in-bad-data-response
Open

fix: surface superstruct validation errors and normalize update-API paths#1546
drlukeangel wants to merge 2 commits into
Thinkmill:mainfrom
drlukeangel:surface-superstruct-errors-in-bad-data-response

Conversation

@drlukeangel

@drlukeangel drlukeangel commented Jun 5, 2026

Copy link
Copy Markdown

Two related fixes for a single user-facing bug

Saving an entry that contains a markdoc field with a relative-path asset reference (e.g. ![](../../assets/blog/x.png)) fails with HTTP 400 and a body of literally "Bad data" — no path, no field, no schema hint.

The cause is two-fold, and this PR addresses both in one place because the second fix is hard to verify without the first.

1. fix(api): surface superstruct validation errors

packages/keystatic/src/api/api-node.ts

The update endpoint's try { s.create(...) } catch { ... } was returning a constant 'Bad data' string and discarding the StructError. That error contains .path, .type, .message, and .failures() — all of which were silently dropped.

This PR widens the catch to inspect the error. When it's a superstruct StructError, the response body becomes JSON:

{
  "error": "Bad data",
  "path": "deletions.0.path",
  "type": "string",
  "message": "Expected a value of type `string`, but received: ...",
  "failures": [
    { "path": ["deletions", 0, "path"], "type": "string", "refinement": "filepath", "message": "..." }
  ]
}

Non-StructError exceptions still return the bare 'Bad data' string (backwards-compatible with anyone reading the body as a plain string). content-type: application/json is set on the structured branch only, so clients can dispatch on the header.

2. fix(app): normalize update-API paths at the wire boundary

packages/keystatic/src/app/path-utils.ts, packages/keystatic/src/app/updating.tsx

The editor builds wire-format paths for asset additions/deletions as:

{basePath}/{slug}/{field}/{relative_asset_path}

For a markdoc body like ![](../../assets/blog/x.png) in src/content/blog/{slug}.mdoc, the wire path becomes:

src/content/blog/{slug}/content/../../assets/blog/x.png

That string was sent to the API verbatim. The API's getIsPathValid refinement rejects any path segment equal to .., so the schema validation fails on additions[].path or deletions[].path. Combined with fix #1 this is now diagnosable; without it the user sees only Bad data.

This PR adds a small POSIX-style path normalizer (normalizePosixPath) and applies it at the two fetch('/api/keystatic/update', { ... }) sites in useUpsertItem and useDeleteItem. Normalization happens at the wire boundary only, so the upstream diff math (additions vs initialFiles to derive deletions) continues to run on the raw, unnormalized paths and the symmetric cancellation between them is preserved. Only the final JSON sent over the wire has .. segments resolved.

Tests

packages/keystatic/src/app/path-utils.test.ts covers:

  • Single .. resolution
  • Consecutive ..
  • . segment removal
  • Consecutive / collapse
  • Leading .. preservation when there is no parent to resolve against
  • Absolute-root behavior (/../a/a)
  • Already-clean relative and absolute paths
  • The realistic Keystatic update case from this bug

Why one PR

Fix #1 is unambiguously good. Fix #2 fixes the user-visible failure but is meaningfully easier to review together with #1, because #1 is the diagnostic that makes the failure shape obvious in the first place. Splitting them risks #2 looking arbitrary without the context #1 provides.

Happy to revise the response shape, the location of the normalizer (path-utils.ts vs inline), the test layout, or anything else.

…bare "Bad data"

The update API's schema validation catch block was returning a constant
"Bad data" string body with no information about which field failed or
why. This made diagnosing routine save failures (mismatched paths,
invalid base64, etc.) extremely difficult — the only path to the actual
error required patching node_modules locally to surface the StructError.

This change widens the catch to inspect the error and, when it is a
superstruct StructError, returns a JSON body with the failing path,
type, message, and per-failure detail. Non-StructError exceptions still
return the original bare "Bad data" string, preserving the existing
behavior for any unexpected error class.

The response structure:

  {
    "error": "Bad data",
    "path": "deletions.0.path",
    "type": "string",
    "message": "Expected a value of type `string`, but received: ...",
    "failures": [{ "path": [...], "type": ..., "message": ..., "refinement": ... }]
  }

Existing clients that read the response body as a plain string for the
generic 400 branch are unaffected. Clients that want richer diagnostics
can parse JSON when the content-type header is application/json.
@drlukeangel drlukeangel requested a review from emmatown as a code owner June 5, 2026 16:12
@changeset-bot

changeset-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 10b0725

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

When an entry contains a markdoc field that references an asset via a
relative path (e.g. `../../assets/blog/x.png`), the editor's wire-format
path for that asset is built as:

  {basePath}/{slug}/{field}/{relative_asset_path}

For the example above the result is:

  src/content/blog/{slug}/content/../../assets/blog/x.png

That string was sent to /api/keystatic/update verbatim. Inside the API,
`getIsPathValid` rejects any path segment equal to `..`, so the
refinement on `additions[].path` / `deletions[].path` fails and the
endpoint returns the generic 400 "Bad data" response. The user sees no
hint that the failure is a path-shape problem.

This change adds a small POSIX path normalizer (`normalizePosixPath`)
and uses it at the two `fetch('/api/keystatic/update')` sites in
`useUpsertItem` and `useDeleteItem`. Normalization happens only at the
wire boundary so the upstream diff math (additions vs initialFiles to
compute deletions) continues to run on the raw, unnormalized paths and
their symmetric cancellation is preserved.

Tests cover the standard normalization cases (single `..`, consecutive
`..`, `.`, double slashes, leading `..` preservation, absolute root
behavior) plus the realistic Keystatic case from the bug.
@drlukeangel drlukeangel changed the title fix(api): surface superstruct validation errors instead of returning bare "Bad data" fix: surface superstruct validation errors and normalize update-API paths Jun 5, 2026
drlukeangel added a commit to drlukeangel/lukeangel that referenced this pull request Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant