Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thin-berries-greet.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tinacms": patch
---

Fix React Error #31 when inserting image from Media Library with custom MediaStore
14 changes: 14 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@

TinaCMS is an open-source headless CMS with visual editing. This monorepo contains the core packages, CLI, admin app, and framework example apps.

## Security & Disclosure

This repo has a public security policy at [`SECURITY.md`](SECURITY.md). Read it before touching code that could surface a vulnerability (auth, media routes, path handling, GraphQL resolvers, anything that accepts untrusted input).

**Hard rules:**

- **Never file security vulnerabilities as public GitHub issues.** Reports go to `security@tina.io`, or as a draft GitHub Security Advisory. Public issues are explicitly forbidden by `SECURITY.md`, regardless of severity.
- **Never put bug specifics in test comments, PR descriptions, or commit messages.** That's public disclosure by a different name. If a test guards against a known-broken behaviour, use neutral wording (e.g. *"Pending upstream fix reported privately per SECURITY.md"*) and keep the internals — error messages, stack traces, affected function names, response-body specifics — out of the comment.
- **Don't commit, push, or draft a public issue if you find a potential vulnerability.** Stop, tell the user what you found and where, and let them decide the disclosure path (email, draft advisory, or public if they judge it's truly benign).

**Scope of the above rules:** every surface an agent might modify — source files, test files, inline comments, commit messages, PR descriptions, linked issue drafts, README snippets.

When writing regression tests that guard against a quietly-reported finding, mark the specific case with `test.fixme` (Playwright) or the framework equivalent, use a generic "pending upstream fix" message, and put the detail in a private channel only.

## Monorepo Structure

```
Expand Down
8 changes: 8 additions & 0 deletions packages/tinacms/src/toolkit/core/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ export interface MediaStore {
* @default false
*/
isStatic?: boolean;

/**
* Converts a Media object to the value stored in a form field.
*
* Typically returns `media.src`. If not implemented, the image field
* plugin falls back to `media.src`.
*/
parse?(media: Media): string;
}

export declare type MediaListOffset = string | number;
Expand Down
186 changes: 186 additions & 0 deletions packages/tinacms/src/toolkit/fields/plugins/image-field-plugin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
import { describe, it, expect, vi } from 'vitest';
import type { Media } from '@toolkit/core/media';

/**
* Tests for the image field plugin's onChange behavior.
*
* The core bug: when a custom MediaStore doesn't implement a `parse()` method,
* the image field plugin's onChange was passing the entire Media object as the
* field value instead of extracting media.src. This caused React Error #31
* ("Objects are not valid as React children") when the object was rendered.
*
* We extract and test the onChange logic directly since the component itself
* requires extensive React context (CMS, forms, etc.) to render.
*/

// Simulate the onChange logic from image-field-plugin.tsx
// This mirrors the fixed code at lines 34-46
function imageFieldOnChange(
media: Media | Media[] | undefined,
cmsMediaStoreParse: ((img: any) => string) | undefined
): any {
if (media) {
const item = Array.isArray(media) ? media[0] : media;
const parsedValue =
typeof cmsMediaStoreParse === 'function'
? cmsMediaStoreParse(media)
: item.src || item;
return parsedValue;
}
return undefined;
}

// Simulate the ORIGINAL (buggy) onChange logic for comparison
function imageFieldOnChangeOriginal(
media: Media | Media[] | undefined,
cmsMediaStoreParse: ((img: any) => string) | undefined
): any {
if (media) {
const parsedValue =
typeof cmsMediaStoreParse === 'function'
? cmsMediaStoreParse(media)
: media; // BUG: passes full object when parse is missing
return parsedValue;
}
return undefined;
}

const mockMedia: Media = {
id: 'test-image-123',
src: 'https://example.com/images/hero.jpg',
filename: 'hero.jpg',
directory: '/images',
type: 'file',
thumbnails: {
'75x75': 'https://example.com/images/hero-75.jpg',
'400x400': 'https://example.com/images/hero-400.jpg',
},
};

const mockMediaNoSrc: Media = {
id: 'test-file-456',
filename: 'document.pdf',
directory: '/docs',
type: 'file',
};

describe('image-field-plugin onChange', () => {
describe('BUG REPRODUCTION: original code passes full Media object when store has no parse()', () => {
it('original code returns the full Media object (causes React Error #31)', () => {
const result = imageFieldOnChangeOriginal(mockMedia, undefined);

// This is the bug - the entire object is returned
expect(result).toBe(mockMedia);
expect(typeof result).toBe('object');
// React would choke trying to render this as a child
expect(result).toHaveProperty('id');
expect(result).toHaveProperty('filename');
expect(result).toHaveProperty('thumbnails');
});
});

describe('FIX: extracts media.src when store has no parse()', () => {
it('returns media.src string when store has no parse()', () => {
const result = imageFieldOnChange(mockMedia, undefined);

expect(result).toBe('https://example.com/images/hero.jpg');
expect(typeof result).toBe('string');
});

it('returns media.src string when store.parse is not a function', () => {
// Simulating cms.media.store.parse being undefined/null
const result = imageFieldOnChange(mockMedia, undefined);

expect(result).toBe('https://example.com/images/hero.jpg');
expect(typeof result).toBe('string');
});

it('uses store.parse() when available (e.g., TinaMediaStore)', () => {
const parseFn = (img: any) => img.src;
const result = imageFieldOnChange(mockMedia, parseFn);

expect(result).toBe('https://example.com/images/hero.jpg');
});

it('handles Media array input (extracts first item)', () => {
const result = imageFieldOnChange([mockMedia], undefined);

expect(result).toBe('https://example.com/images/hero.jpg');
expect(typeof result).toBe('string');
});

it('falls back to full item when src is undefined', () => {
const result = imageFieldOnChange(mockMediaNoSrc, undefined);

// When src is undefined, falls back to the item itself
// This preserves backward compatibility for edge cases
expect(result).toBe(mockMediaNoSrc);
});

it('returns undefined when media is undefined', () => {
const result = imageFieldOnChange(undefined, undefined);
expect(result).toBeUndefined();
});

it('custom store parse can return custom path format', () => {
// Some custom stores might return a relative path
const customParse = (img: any) =>
`/uploads/${img.directory}/${img.filename}`;
const result = imageFieldOnChange(mockMedia, customParse);

expect(result).toBe('/uploads//images/hero.jpg');
expect(typeof result).toBe('string');
});
});
});

describe('MDX image toolbar button onSelect', () => {
/**
* Simulates the fixed onSelect logic from image-toolbar-button.tsx
* that prepares the media object before passing to insertImg.
*/
function toolbarOnSelect(
media: Media,
cmsMediaStoreParse: ((img: any) => string) | undefined
): Media {
const src =
typeof cmsMediaStoreParse === 'function'
? cmsMediaStoreParse(media)
: media.src;
return { ...media, src: src || media.src };
}

it('passes media with correct src when store has no parse()', () => {
const result = toolbarOnSelect(mockMedia, undefined);

expect(result.src).toBe('https://example.com/images/hero.jpg');
expect(typeof result.src).toBe('string');
// Should still carry other properties for insertImg
expect(result.filename).toBe('hero.jpg');
});

it('uses store.parse() to resolve src when available', () => {
const parseFn = (img: any) => img.src;
const result = toolbarOnSelect(mockMedia, parseFn);

expect(result.src).toBe('https://example.com/images/hero.jpg');
});

it('custom store parse can transform the src', () => {
// A custom store might return a proxy URL
const customParse = (img: any) => `/api/media/proxy?src=${img.src}`;
const result = toolbarOnSelect(mockMedia, customParse);

expect(result.src).toBe(
'/api/media/proxy?src=https://example.com/images/hero.jpg'
);
});

it('preserves original src when parse returns falsy', () => {
const badParse = (_img: any) => '';
const result = toolbarOnSelect(mockMedia, badParse);

// Falls back to media.src when parse returns empty string
expect(result.src).toBe('https://example.com/images/hero.jpg');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@ export const ImageField = wrapFieldsWithMeta<InputProps, ImageProps>(

async function onChange(media?: Media | Media[]) {
if (media) {
const item = Array.isArray(media) ? media[0] : media;
const parsedValue =
// @ts-ignore
typeof cms?.media?.store?.parse === 'function'
? // @ts-ignore
cms.media.store.parse(media)
: media;
? cms.media.store.parse(item)
: item.src || item;

props.input.onChange(parsedValue);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ const useImageToolbarButton = (state) => {
allowDelete: true,
directory: '',
onSelect: (media) => {
insertImg(editor, media);
// Ensure media.src is a string, falling back to parse if available
const src =
typeof cms?.media?.store?.parse === 'function'
? cms.media.store.parse(media)
: media.src;
insertImg(editor, { ...media, src: src || media.src });
},
});
};
Expand Down
7 changes: 6 additions & 1 deletion playwright/tina-playwright/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@ yarn-error.log*
/blob-report/
/playwright/.cache/

/playwright-test-results.json
/playwright-test-results.json

# Ignore generated playwright test files, but not the source files.
content/*/playwright-*
!content/*/playwright-filter-*
public/uploads/playwright-*
43 changes: 43 additions & 0 deletions playwright/tina-playwright/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,46 @@ You can also run test in UI mode for test debugging purpose
⚠️ Note

- Create blog post test required the example project to have no blog post in order to past the test(Dont create any blog post manually in this tinacms project)

## Projects

`playwright.config.ts` defines three projects:

| Project | Target | Runs browser? |
|---|---|---|
| `chromium` | UI tests against the Next.js dev server on `http://localhost:3000` | Yes |
| `posthog` | PostHog telemetry tests; depends on `chromium` | Yes |
| `api` | GraphQL / HTTP integration tests against the TinaCMS dev server on `http://localhost:4001` | No |

Run a single project:

```bash
npx playwright test --project=api # HTTP-boundary tests, no browser
npx playwright test --project=chromium # UI tests
npx playwright test # everything
```

## API test suite (`tests/api/`)

Integration tests hitting the live dev server at the HTTP boundary. No browser, no mocks — real LevelDB, real filesystem, real request/response.

| Spec | Covers |
|---|---|
| [`document-crud.spec.ts`](tests/api/document-crud.spec.ts) | `createDocument` / `updateDocument` / `deleteDocument` lifecycle across `.md` / `.mdx` / `.json` |
| [`filtering.spec.ts`](tests/api/filtering.spec.ts) | `postConnection(filter:)` against real LevelDB |
| [`pagination.spec.ts`](tests/api/pagination.spec.ts) | Cursor pagination — `first`, `after`, `hasNextPage` |
| [`sorting.spec.ts`](tests/api/sorting.spec.ts) | Index freshness on create/update/delete; ascending/descending ordering |
| [`search.spec.ts`](tests/api/search.spec.ts) | `/searchIndex` route — field indexing and query precision |
| [`security.spec.ts`](tests/api/security.spec.ts) | Path-traversal rejection on `createDocument` and `/media/upload`, no filesystem leak in error bodies |
| [`media.spec.ts`](tests/api/media.spec.ts) | `/media` upload → list → delete round-trip |
| [`concurrency.spec.ts`](tests/api/concurrency.spec.ts) | `AsyncLock`-serialised mutations survive parallel contention without index corruption |

### Shared infrastructure

- [`fixtures/api-context.ts`](fixtures/api-context.ts) — pre-configured `APIRequestContext` for the GraphQL and `/media` endpoints
- [`fixtures/test-content.ts`](fixtures/test-content.ts) — extends `api-context` with two cleanup fixtures:
- `contentCleanup.track(collection, relativePath)` — auto-deletes documents created during a test
- `mediaCleanup.track(relativePath)` — auto-deletes uploaded media
Both fixtures check on-disk state first so teardown doesn't double-delete files the test itself already removed.
- [`utils/graphql.ts`](utils/graphql.ts) — shared GraphQL mutation strings plus `createDocument` / `updateDocument` / `deleteDocument` / `gqlRequest` helpers; exports a `CollectionName` union that matches `tina/collections/`
- [`utils/media.ts`](utils/media.ts) — `uploadMedia` / `listMedia` / `deleteMedia` helpers; `uploadMedia` accepts a `rawPath: true` option for pre-encoded path traversal vectors
7 changes: 4 additions & 3 deletions playwright/tina-playwright/fixtures/api-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ type ApiFixtures = {
*/
export const test = base.extend<ApiFixtures>({
apiContext: async ({ playwright }, use) => {
// No extraHTTPHeaders: Playwright derives Content-Type automatically from
// the request body (application/json for `data`, multipart/form-data with
// boundary for `multipart`). A hard-coded default would break media
// uploads by clobbering the multipart boundary.
const context = await playwright.request.newContext({
baseURL: process.env.GRAPHQL_URL ?? "http://localhost:4001",
extraHTTPHeaders: {
"Content-Type": "application/json",
},
});

await use(context);
Expand Down
Loading
Loading