From 4d0c37a8a50b211b7c5070c370faa369ee5d260d Mon Sep 17 00:00:00 2001 From: "Josh Berman [SSW]" <137844305+joshbermanssw@users.noreply.github.com> Date: Fri, 8 May 2026 17:30:00 +1000 Subject: [PATCH] feat(cli): tinacms dev & build support for localContentPath (no duplicate content-repo writes) (#6738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes [tinacms/tinacloud#3295](https://github.com/tinacms/tinacloud/issues/3295). ## Before When a project sets `localContentPath` to point at a sibling content repo, every `tinacms dev` and `tinacms build` writes the generated artifacts (`_schema.json`, `_graphql.json`, `_lookup.json`, `tina-lock.json`) **twice**: once to the generator's `tina/__generated__/`, and again to the content repo. Editors then see a `tina/` folder full of build output land in their content repo on every commit. The content repo also has to *already contain* a `tina/` folder or the CLI errors out — so editors can't keep their content repo clean even if they wanted to. ## After Generated artifacts go only to the generator's `tina/__generated__/`. The content repo is pure content; no `tina/` folder required, no build output committed, no duplicate `tina-lock.json`. Two pieces make that work: - The duplicate-write branches in `Codegen` and `DevCommand` are gone. - `FilesystemBridge` routes any path starting with `tina/__generated__/` or `.tina/__generated__/` to `rootPath` (the generator); everything else still uses `outputPath` (the content root). Without this, schema/graphql/lookup reads after the duplicate-write removal would `ENOENT` on the content side. ## Testing I had my whole regular setup locally; - [ ] tinacloud running locally on main branch - [ ] created a seperate content repo project pointing to SSW.Rules.Josh and SSW.Rules.Content.Test.JB - [ ] SSW.Rules.Josh had tina config file ovverides pointing to the local instance of TinaCloud + .env set up - [ ] ran pnpm dev and pnpm build locally to ensure that tina files are only written to generator repo - [ ] deployed to vercel and tested editing - [ ] SSW.Rules.Josh is using the tagged tina versions (check out its repo if u want) Screenshot 2026-05-06 at 4 30 33 pm **Figure: my deployed site url** --- ## A bit more **Public-API changes** (covered in the changeset migration notes): - \`ConfigManager.generatedFolderPathContentRepo\` removed — use \`generatedFolderPath\`. - \`ConfigManager.getTinaFolderPath\`'s \`isContentRoot\` option removed. - \`FilesystemBridge\` routing for \`tina/__generated__/\` resolves to \`rootPath\` in multi-repo setups (custom bridge subclasses may need updates). - Existing multi-repo users will have a stale \`tina/\` folder in their content repo after upgrade — safe to delete. **Bonus fix rolled in.** Generated \`client.ts\` / \`database-client.ts\` now import \`./types\` (extensionless) for TS projects instead of \`./types.ts\` — avoids requiring \`allowImportingTsExtensions: true\` in consumer tsconfigs (broke under Next.js 15.5+ defaults). JS projects still import \`./types.js\` (Node ESM requires it). --------- Co-authored-by: Claude Opus 4.7 (1M context) --- ...ld-localcontentpath-no-duplicate-writes.md | 21 ++ .../cli/src/next/codegen/index.test.ts | 67 ++++- .../@tinacms/cli/src/next/codegen/index.ts | 12 +- .../src/next/commands/build-command/index.ts | 11 + .../src/next/commands/dev-command/index.ts | 24 +- .../@tinacms/cli/src/next/config-manager.ts | 21 +- .../localcontentpath-orchestration.test.ts | 232 ++++++++++++++++++ .../graphql/src/database/bridge/filesystem.ts | 54 +++- .../tests/filesystem-bridge/index.test.ts | 135 +++++++++- .../@tinacms/metrics/src/interfaces/index.ts | 15 +- 10 files changed, 537 insertions(+), 55 deletions(-) create mode 100644 .changeset/tinacms-dev-build-localcontentpath-no-duplicate-writes.md create mode 100644 packages/@tinacms/cli/src/next/localcontentpath-orchestration.test.ts diff --git a/.changeset/tinacms-dev-build-localcontentpath-no-duplicate-writes.md b/.changeset/tinacms-dev-build-localcontentpath-no-duplicate-writes.md new file mode 100644 index 0000000000..eb1446c830 --- /dev/null +++ b/.changeset/tinacms-dev-build-localcontentpath-no-duplicate-writes.md @@ -0,0 +1,21 @@ +--- +"@tinacms/cli": minor +"@tinacms/graphql": minor +"@tinacms/metrics": minor +--- + +Stop writing generated files (`_schema.json`, `_graphql.json`, `_lookup.json`, `tina-lock.json`) to the content repo when `localContentPath` is set. Generated files now live only in the generator repo's `tina/__generated__/`. The content repo is no longer required to contain a `tina/` folder. `FilesystemBridge.get` / `put` / `delete` now route `tina/__generated__/` and `.tina/__generated__/` paths to `rootPath` (the generator) instead of `outputPath` (the content root). Closes [tinacms/tinacloud#3295](https://github.com/tinacms/tinacloud/issues/3295). + +### ⚠️ Rollout gate + +**This release must not be promoted to the `@latest` dist-tag until TinaCloud prod has deployed [tinacms/tinacloud#3403](https://github.com/tinacms/tinacloud/issues/3403).** Pre-#3403 TinaCloud reads `tina-lock.json` from the content repo on generator pushes; shipping this change before the server-side fix breaks every existing multi-repo user's indexing. + +### Migration notes for existing multi-repo projects + +After upgrading (and once TinaCloud prod is on #3403): + +- **Stale `tina/` folder in your content repo.** Pre-upgrade builds committed `tina/__generated__/*` and `tina/tina-lock.json` to the content repo. Nothing updates or reads those files any more. They are safe — and recommended — to delete from the content repo in a single cleanup commit. +- **`ConfigManager.generatedFolderPathContentRepo` is removed.** If any custom CLI code, plugins, or scripts referenced this field, they will fail at type-check or runtime. Use `generatedFolderPath` — it has always been the generator-relative path. +- **`ConfigManager.getTinaFolderPath` no longer accepts an `isContentRoot` option.** The content root never needs a `tina/` folder now, so the option was removed. If any custom code called `getTinaFolderPath(path, { isContentRoot: true })`, drop the second argument. +- **`FilesystemBridge` behavior change for `tina/__generated__/` paths.** In multi-repo setups, bridge reads/writes of paths under `tina/__generated__/` or `.tina/__generated__/` now resolve against the generator (`rootPath`) rather than the content repo (`outputPath`). If you have custom bridge subclasses or code that relied on these paths resolving to the content repo, update it. +- **Generated `client.ts` / `database-client.ts` now import `./types` extensionless** (was `./types.ts`) for TypeScript projects. Avoids requiring `allowImportingTsExtensions: true` in consumer tsconfigs, which broke the build under Next.js 15.5+ defaults. JS projects still import `./types.js` (Node ESM requires the extension). diff --git a/packages/@tinacms/cli/src/next/codegen/index.test.ts b/packages/@tinacms/cli/src/next/codegen/index.test.ts index 6b9fbd432d..62ea160a30 100644 --- a/packages/@tinacms/cli/src/next/codegen/index.test.ts +++ b/packages/@tinacms/cli/src/next/codegen/index.test.ts @@ -16,6 +16,7 @@ jest.mock('esbuild', () => ({ transform: jest.fn().mockResolvedValue({ code: '' }), })); +import path from 'path'; import * as stripModule from './stripSearchTokenFromConfig'; import { Codegen } from './index'; @@ -33,13 +34,19 @@ describe('Codegen.genClient', () => { return instance; } - it('emits ./types.ts import for TypeScript projects', async () => { + it('emits extensionless ./types import for TypeScript projects', async () => { + // Avoids requiring `allowImportingTsExtensions: true` in consumer + // tsconfigs. Modern TS module resolution (Bundler / NodeNext) resolves + // `./types` to `./types.ts` cleanly. The previous `./types.ts` import + // broke under stricter Next.js / TS defaults (see #6062 follow-up). const { clientString } = await makeInstance(true).genClient(); - expect(clientString).toContain('from "./types.ts"'); - expect(clientString).not.toMatch(/from ["']\.\/types["']/); + expect(clientString).toContain('from "./types"'); + expect(clientString).not.toMatch(/from ["']\.\/types\.ts["']/); + expect(clientString).not.toMatch(/from ["']\.\/types\.js["']/); }); it('emits ./types.js import for non-TypeScript projects', async () => { + // Node ESM strictly requires the extension on relative imports. const { clientString } = await makeInstance(false).genClient(); expect(clientString).toContain('from "./types.js"'); expect(clientString).not.toMatch(/from ["']\.\/types["']/); @@ -58,10 +65,11 @@ describe('Codegen.genDatabaseClient', () => { return instance; } - it('emits ./types.ts import for TypeScript projects', async () => { + it('emits extensionless ./types import for TypeScript projects', async () => { const result = await makeInstance(true).genDatabaseClient(); - expect(result).toContain('from "./types.ts"'); - expect(result).not.toMatch(/from ["']\.\/types["']/); + expect(result).toContain('from "./types"'); + expect(result).not.toMatch(/from ["']\.\/types\.ts["']/); + expect(result).not.toMatch(/from ["']\.\/types\.js["']/); }); it('emits ./types.js import for non-TypeScript projects', async () => { @@ -106,7 +114,6 @@ describe('Codegen.execute integration', () => { } as any; instance.configManager = { generatedFolderPath: '/fake/tina/__generated__', - generatedFolderPathContentRepo: '/fake/tina/__generated__', generatedSchemaJSONPath: '/fake/tina/__generated__/_schema.json', generatedQueriesFilePath: '/fake/tina/__generated__/queries.gql', generatedFragmentsFilePath: '/fake/tina/__generated__/frags.gql', @@ -115,7 +122,6 @@ describe('Codegen.execute integration', () => { token: 'tok', clientId: 'cid', }, - hasSeparateContentRoot: () => false, shouldSkipSDK: () => true, getTinaGraphQLVersion: () => ({ fullVersion: '1.0.0', @@ -172,4 +178,49 @@ describe('Codegen.execute integration', () => { expect(writtenData.config).toEqual(SAFE_RESULT); expect(JSON.stringify(writtenData)).not.toContain('secret-token'); }); + + it('writes each generated config file exactly once (never duplicates to a content repo path)', async () => { + const codegen = stubCodegen(); + const fs = jest.requireMock('fs-extra'); + (fs.outputFile as jest.Mock).mockClear(); + + await codegen.execute(); + + for (const fileName of ['_schema.json', '_graphql.json', '_lookup.json']) { + const calls = (fs.outputFile as jest.Mock).mock.calls.filter( + ([filePath]: [string]) => filePath.endsWith(fileName) + ); + expect(calls).toHaveLength(1); + expect(calls[0][0]).toBe( + path.join(codegen.configManager.generatedFolderPath, fileName) + ); + } + }); + + it('still writes exactly once when the project has a separate content root', async () => { + // Strengthens the previous test: even when hasSeparateContentRoot returns + // true (the multi-repo flag the duplicate-write block used to gate on), + // we should still see exactly one write per generated file. Pins that the + // duplicate-write code is gone, not just unreachable in the default mock. + const codegen = stubCodegen(); + (codegen.configManager as any).hasSeparateContentRoot = () => true; + (codegen.configManager as any).contentRootPath = '/fake-content-root'; + + const fs = jest.requireMock('fs-extra'); + (fs.outputFile as jest.Mock).mockClear(); + + await codegen.execute(); + + for (const fileName of ['_schema.json', '_graphql.json', '_lookup.json']) { + const calls = (fs.outputFile as jest.Mock).mock.calls.filter( + ([filePath]: [string]) => filePath.endsWith(fileName) + ); + expect(calls).toHaveLength(1); + expect(calls[0][0]).toBe( + path.join(codegen.configManager.generatedFolderPath, fileName) + ); + // Specifically: no write under the content-root path. + expect(calls[0][0]).not.toContain('fake-content-root'); + } + }); }); diff --git a/packages/@tinacms/cli/src/next/codegen/index.ts b/packages/@tinacms/cli/src/next/codegen/index.ts index c0056c8fd2..e239e3c104 100644 --- a/packages/@tinacms/cli/src/next/codegen/index.ts +++ b/packages/@tinacms/cli/src/next/codegen/index.ts @@ -76,14 +76,6 @@ export class Codegen { ); await fs.ensureFile(filePath); await fs.outputFile(filePath, data); - if (this.configManager.hasSeparateContentRoot()) { - const filePath = path.join( - this.configManager.generatedFolderPathContentRepo, - fileName - ); - await fs.ensureFile(filePath); - await fs.outputFile(filePath, data); - } } async removeGeneratedFilesIfExists() { @@ -297,7 +289,7 @@ export class Codegen { import { resolve } from "@tinacms/datalayer"; import type { TinaClient } from "tinacms/dist/client"; -import { queries } from "${this.configManager.isUsingTs() ? './types.ts' : './types.js'}"; +import { queries } from "${this.configManager.isUsingTs() ? './types' : './types.js'}"; import database from "../database"; export async function databaseRequest({ query, variables, user }) { @@ -365,7 +357,7 @@ export default databaseClient; const apiURL = this.getApiURL(); const clientString = `import { createClient } from "tinacms/dist/client"; -import { queries } from "${this.configManager.isUsingTs() ? './types.ts' : './types.js'}"; +import { queries } from "${this.configManager.isUsingTs() ? './types' : './types.js'}"; export const client = createClient({ ${ this.noClientBuildCache === false ? `cacheDir: '${normalizePath( diff --git a/packages/@tinacms/cli/src/next/commands/build-command/index.ts b/packages/@tinacms/cli/src/next/commands/build-command/index.ts index ee9eae22c6..58afa881b1 100644 --- a/packages/@tinacms/cli/src/next/commands/build-command/index.ts +++ b/packages/@tinacms/cli/src/next/commands/build-command/index.ts @@ -2,6 +2,7 @@ import crypto from 'crypto'; import path from 'path'; import { ChangeType, diff } from '@graphql-inspector/core'; import { type Database, FilesystemBridge, buildSchema } from '@tinacms/graphql'; +import { Telemetry } from '@tinacms/metrics'; import { parseURL } from '@tinacms/schema-tools'; import { type SearchClient, @@ -132,6 +133,16 @@ export class BuildCommand extends BaseCommand { ); process.exit(1); } + + // Track localContentPath usage so we can measure adoption of the + // multi-repo separation. + const telemetry = new Telemetry({ disabled: this.noTelemetry }); + await telemetry.submitRecord({ + event: { + name: 'tinacms:cli:build:invoke', + hasLocalContentPath: Boolean(configManager.config.localContentPath), + }, + }); if (localContentOnly && !this.localOption) { const config = configManager.config; const missing = []; diff --git a/packages/@tinacms/cli/src/next/commands/dev-command/index.ts b/packages/@tinacms/cli/src/next/commands/dev-command/index.ts index 255e0ad863..8d61babf18 100644 --- a/packages/@tinacms/cli/src/next/commands/dev-command/index.ts +++ b/packages/@tinacms/cli/src/next/commands/dev-command/index.ts @@ -1,5 +1,6 @@ import path from 'path'; import { Database, FilesystemBridge, buildSchema } from '@tinacms/graphql'; +import { Telemetry } from '@tinacms/metrics'; import { LocalSearchIndexClient, SearchIndexer } from '@tinacms/search'; import AsyncLock from 'async-lock'; import chokidar from 'chokidar'; @@ -78,6 +79,19 @@ export class DevCommand extends BaseCommand { try { await configManager.processConfig(); if (firstTime) { + // Track localContentPath usage so we can measure adoption of the + // multi-repo separation. Fire once per `tinacms dev` invocation, not + // per reload. + const telemetry = new Telemetry({ disabled: this.noTelemetry }); + await telemetry.submitRecord({ + event: { + name: 'tinacms:cli:dev:invoke', + hasLocalContentPath: Boolean( + configManager.config.localContentPath + ), + }, + }); + database = await createAndInitializeDatabase( configManager, Number(this.datalayerPort) @@ -123,16 +137,6 @@ export class DevCommand extends BaseCommand { path.join(configManager.tinaFolderPath, tinaLockFilename), tinaLockContent ); - - if (configManager.hasSeparateContentRoot()) { - const rootPath = await configManager.getTinaFolderPath( - configManager.contentRootPath, - { isContentRoot: true } - ); - const filePath = path.join(rootPath, tinaLockFilename); - await fs.ensureFile(filePath); - await fs.outputFile(filePath, tinaLockContent); - } } await this.indexContentWithSpinner({ diff --git a/packages/@tinacms/cli/src/next/config-manager.ts b/packages/@tinacms/cli/src/next/config-manager.ts index e66fb70c8a..029fdc7f7e 100644 --- a/packages/@tinacms/cli/src/next/config-manager.ts +++ b/packages/@tinacms/cli/src/next/config-manager.ts @@ -7,10 +7,8 @@ import type { Loader } from 'esbuild'; import { Config } from '@tinacms/schema-tools'; import * as dotenv from 'dotenv'; import normalizePath from 'normalize-path'; -import chalk from 'chalk'; -import { logger } from '../logger'; import { createRequire } from 'module'; -import { stripNativeTrailingSlash } from '../utils/path'; +import { logger } from '../logger'; import { warnText } from '../utils/theme'; import { resolveContentRootPath } from './resolve-content-root'; @@ -34,7 +32,6 @@ export class ConfigManager { envFilePath: string; generatedCachePath: string; generatedFolderPath: string; - generatedFolderPathContentRepo: string; generatedGraphQLGQLPath: string; generatedGraphQLJSONPath: string; generatedSchemaJSONPath: string; @@ -260,22 +257,13 @@ export class ConfigManager { localContentPath: this.config.localContentPath, }); - this.generatedFolderPathContentRepo = path.join( - await this.getTinaFolderPath(this.contentRootPath, { - isContentRoot: this.hasSeparateContentRoot(), - }), - GENERATED_FOLDER - ); this.spaMainPath = require.resolve('@tinacms/app'); this.spaRootPath = path.join(this.spaMainPath, '..', '..'); // ================= // End of paths that depend on the config file } - async getTinaFolderPath( - rootPath: string, - { isContentRoot }: { isContentRoot?: boolean } = {} - ) { + async getTinaFolderPath(rootPath: string) { const tinaFolderPath = path.join(rootPath, TINA_FOLDER); const tinaFolderExists = await fs.pathExists(tinaFolderPath); if (tinaFolderExists) { @@ -288,11 +276,6 @@ export class ConfigManager { this.isUsingLegacyFolder = true; return legacyFolderPath; } - if (isContentRoot) { - throw new Error( - `Unable to find a ${chalk.cyan('tina/')} folder in your content root at ${chalk.cyan(rootPath)}. When using localContentPath, the content directory must contain a ${chalk.cyan('tina/')} folder for generated files. Create one with: mkdir ${path.join(rootPath, TINA_FOLDER)}` - ); - } throw new Error( `Unable to find Tina folder, if you're working in folder outside of the Tina config be sure to specify --rootPath` ); diff --git a/packages/@tinacms/cli/src/next/localcontentpath-orchestration.test.ts b/packages/@tinacms/cli/src/next/localcontentpath-orchestration.test.ts new file mode 100644 index 0000000000..9805ac567b --- /dev/null +++ b/packages/@tinacms/cli/src/next/localcontentpath-orchestration.test.ts @@ -0,0 +1,232 @@ +// End-to-end orchestration test for `localContentPath`. +// +// Bridge unit tests (in @tinacms/graphql) pin the routing rule in isolation: +// given a (rootPath, contentRootPath) pair, generated paths go to rootPath and +// content paths go to contentRootPath. resolve-content-root.test.ts pins the +// resolver that turns config + on-disk state into a contentRootPath. +// +// What neither covers is the wiring step: a project sets `localContentPath`, +// `resolveContentRootPath` produces an absolute content path, and a +// FilesystemBridge constructed with `(rootPath, contentRootPath)` then routes +// reads and writes correctly. A future refactor that swapped the bridge +// constructor args, dropped the resolver call, or quietly fed `rootPath` into +// both bridge slots would slip past the unit tests but get caught here. +// +// Real fs, real bridge — only mocks needed are the same chalk + logger stubs +// the resolve-content-root.test.ts file uses (chalk v5 is ESM-only and can't +// be loaded under jest's CommonJS runtime). + +jest.mock('../logger', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +jest.mock('chalk', () => { + const identity = (s: string) => s; + return { + __esModule: true, + default: { yellow: identity, cyan: identity, red: identity }, + yellow: identity, + cyan: identity, + red: identity, + }; +}); + +import os from 'os'; +import path from 'path'; +import fs from 'fs-extra'; +// Import the bridge from source rather than the built package entry point. +// Jest's CLI-package resolver doesn't follow the `exports` field of +// @tinacms/graphql under `useESM: true`, and importing the built dist directly +// hits a CommonJS / ESM boundary. Sourcing from `../graphql/src` keeps the +// test on the current code regardless of build state. +import { FilesystemBridge } from '../../../graphql/src/database/bridge/filesystem'; +import { resolveContentRootPath } from './resolve-content-root'; + +describe('localContentPath orchestration (resolveContentRootPath + FilesystemBridge)', () => { + let parentDir: string; + let generatorDir: string; + let contentDir: string; + let tinaFolderPath: string; + let tinaConfigFilePath: string; + + beforeEach(async () => { + parentDir = path.join( + os.tmpdir(), + `tinacms-orchestration-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); + generatorDir = path.join(parentDir, 'generator-repo'); + contentDir = path.join(parentDir, 'content-repo'); + tinaFolderPath = path.join(generatorDir, 'tina'); + tinaConfigFilePath = path.join(tinaFolderPath, 'config.ts'); + + await fs.mkdirp(tinaFolderPath); + // Sentinel config file path — resolver only uses it for warning messages. + await fs.outputFile( + tinaConfigFilePath, + 'export default { build: {}, schema: {} } as any\n' + ); + }); + + afterEach(async () => { + await fs.remove(parentDir); + }); + + test('localContentPath pointing at an existing sibling routes content to that sibling', async () => { + await fs.mkdirp(contentDir); + + const contentRootPath = await resolveContentRootPath({ + rootPath: generatorDir, + tinaFolderPath, + tinaConfigFilePath, + localContentPath: '../../content-repo', + }); + + expect(contentRootPath).toBe(contentDir); + + const bridge = new FilesystemBridge(generatorDir, contentRootPath); + + await bridge.put('tina/__generated__/_schema.json', '{"v":1}'); + await bridge.put('tina/__generated__/_graphql.json', '{"v":1}'); + await bridge.put('tina/__generated__/_lookup.json', '{"v":1}'); + await bridge.put('posts/hello.md', '# hello'); + + expect( + await fs.pathExists( + path.join(generatorDir, 'tina/__generated__/_schema.json') + ) + ).toBe(true); + expect( + await fs.pathExists( + path.join(generatorDir, 'tina/__generated__/_graphql.json') + ) + ).toBe(true); + expect( + await fs.pathExists( + path.join(generatorDir, 'tina/__generated__/_lookup.json') + ) + ).toBe(true); + expect(await fs.pathExists(path.join(contentDir, 'posts/hello.md'))).toBe( + true + ); + + // Content repo must remain free of generated artifacts. + expect(await fs.pathExists(path.join(contentDir, 'tina'))).toBe(false); + // Generator repo must not absorb content files. + expect(await fs.pathExists(path.join(generatorDir, 'posts/hello.md'))).toBe( + false + ); + }); + + test('reads of generated artifacts come from the generator even if the content repo has shadow copies', async () => { + // Pins the asymmetry: a stale `tina/__generated__/` left behind in the + // content repo (a real-world condition for projects upgrading from the + // old multi-repo model) must not be read from. Generator wins. + await fs.mkdirp(contentDir); + await fs.outputFile( + path.join(generatorDir, 'tina/__generated__/_lookup.json'), + '{"source":"generator"}' + ); + await fs.outputFile( + path.join(contentDir, 'tina/__generated__/_lookup.json'), + '{"source":"content-stale"}' + ); + + const contentRootPath = await resolveContentRootPath({ + rootPath: generatorDir, + tinaFolderPath, + tinaConfigFilePath, + localContentPath: '../../content-repo', + }); + const bridge = new FilesystemBridge(generatorDir, contentRootPath); + + const result = await bridge.get('tina/__generated__/_lookup.json'); + expect(result).toBe('{"source":"generator"}'); + }); + + test('localContentPath pointing at a missing sibling falls back, and the bridge degrades to single-repo behaviour', async () => { + // contentDir intentionally NOT created. + const contentRootPath = await resolveContentRootPath({ + rootPath: generatorDir, + tinaFolderPath, + tinaConfigFilePath, + localContentPath: '../../content-repo', + }); + + expect(contentRootPath).toBe(generatorDir); + + const bridge = new FilesystemBridge(generatorDir, contentRootPath); + await bridge.put('posts/hello.md', '# hello'); + + // With contentRootPath === rootPath, content lands in the generator — + // single-repo behaviour. The fact that the user had localContentPath + // configured doesn't reroute writes if the directory doesn't exist. + expect(await fs.pathExists(path.join(generatorDir, 'posts/hello.md'))).toBe( + true + ); + }); + + test('omitted localContentPath wires up as a single-repo project', async () => { + const contentRootPath = await resolveContentRootPath({ + rootPath: generatorDir, + tinaFolderPath, + tinaConfigFilePath, + localContentPath: undefined, + }); + + expect(contentRootPath).toBe(generatorDir); + + const bridge = new FilesystemBridge(generatorDir, contentRootPath); + await bridge.put('tina/__generated__/_schema.json', '{"v":1}'); + await bridge.put('posts/hello.md', '# hello'); + + expect( + await fs.pathExists( + path.join(generatorDir, 'tina/__generated__/_schema.json') + ) + ).toBe(true); + expect(await fs.pathExists(path.join(generatorDir, 'posts/hello.md'))).toBe( + true + ); + }); + + test('swapped bridge constructor args route to the wrong repo (regression sentinel)', async () => { + // Documents the failure mode kulesy called out: if a future refactor + // calls `new FilesystemBridge(contentRootPath, rootPath)` instead of + // `(rootPath, contentRootPath)`, generated artifacts land in the content + // repo and content lands in the generator. This test asserts that the + // swap is observably wrong, so the orchestration tests above (which use + // the correct order) carry meaning. + await fs.mkdirp(contentDir); + + const contentRootPath = await resolveContentRootPath({ + rootPath: generatorDir, + tinaFolderPath, + tinaConfigFilePath, + localContentPath: '../../content-repo', + }); + + // Intentionally swapped: contentRootPath first, generator second. + const wrongOrderBridge = new FilesystemBridge( + contentRootPath, + generatorDir + ); + await wrongOrderBridge.put('tina/__generated__/_schema.json', '{"v":1}'); + await wrongOrderBridge.put('posts/hello.md', '# hello'); + + // Generated lands in content repo (wrong) — this is the regression a + // swap would silently introduce. + expect( + await fs.pathExists( + path.join(contentDir, 'tina/__generated__/_schema.json') + ) + ).toBe(true); + // Content lands in generator (wrong). + expect(await fs.pathExists(path.join(generatorDir, 'posts/hello.md'))).toBe( + true + ); + }); +}); diff --git a/packages/@tinacms/graphql/src/database/bridge/filesystem.ts b/packages/@tinacms/graphql/src/database/bridge/filesystem.ts index 865cf9c22f..241c4447e2 100644 --- a/packages/@tinacms/graphql/src/database/bridge/filesystem.ts +++ b/packages/@tinacms/graphql/src/database/bridge/filesystem.ts @@ -78,11 +78,25 @@ function assertWithinBase(filepath: string, baseDir: string): string { return resolved; } +const GENERATED_PATH_PREFIXES = ['tina/__generated__/', '.tina/__generated__/']; + +function isGeneratedPath(filepath: string): boolean { + const normalized = filepath.replace(/\\/g, '/'); + return GENERATED_PATH_PREFIXES.some((prefix) => + normalized.startsWith(prefix) + ); +} + /** * This is the bridge from whatever datasource we need for I/O. * The basic example here is for the filesystem, one is needed * for GitHub has well. * + * When `outputPath` differs from `rootPath` (multi-repo: generator + + * sibling content repo), paths under `tina/__generated__/` still resolve + * against `rootPath` — schema/graphql/lookup artifacts live only in the + * generator. Everything else (content files) resolves against `outputPath`. + * * @security All public methods validate their `filepath` / `pattern` * argument via `assertWithinBase` before performing any I/O. If you add a * new method that accepts a path, you MUST validate it the same way. @@ -96,6 +110,37 @@ export class FilesystemBridge implements Bridge { this.outputPath = outputPath ? path.resolve(outputPath) : this.rootPath; } + // Picks the base directory for a given path. The `assertWithinBase` check + // in delete/get/put still runs against the chosen base, so a path like + // `tina/__generated__/../../escape.txt` is still rejected — just rejected + // against `rootPath` rather than `outputPath`. + private baseFor(filepath: string): string { + return isGeneratedPath(filepath) ? this.rootPath : this.outputPath; + } + + // Defense-in-depth for generated-path routing: assertWithinBase already + // rejects paths that escape rootPath, but a path like + // `tina/__generated__/../../.env` would resolve to `/.env` — + // technically inside rootPath but outside the generated subtree the + // routing is supposed to grant access to. Verify the resolved path stays + // inside the matching generated subdirectory. + private assertGeneratedSubtree(filepath: string, resolved: string) { + if (!isGeneratedPath(filepath)) return; + const normalized = filepath.replace(/\\/g, '/'); + const subdir = normalized.startsWith('.tina/__generated__/') + ? '.tina/__generated__' + : 'tina/__generated__'; + const generatedRoot = path.resolve(path.join(this.rootPath, subdir)); + if ( + resolved !== generatedRoot && + !resolved.startsWith(generatedRoot + path.sep) + ) { + throw new Error( + `Path traversal detected: "${filepath}" routed via generated prefix but resolved outside ${generatedRoot}` + ); + } + } + public async glob(pattern: string, extension: string) { const basePath = assertWithinBase(pattern, this.outputPath); const items = await fg( @@ -112,18 +157,21 @@ export class FilesystemBridge implements Bridge { } public async delete(filepath: string) { - const resolved = assertWithinBase(filepath, this.outputPath); + const resolved = assertWithinBase(filepath, this.baseFor(filepath)); + this.assertGeneratedSubtree(filepath, resolved); await fs.remove(resolved); } public async get(filepath: string) { - const resolved = assertWithinBase(filepath, this.outputPath); + const resolved = assertWithinBase(filepath, this.baseFor(filepath)); + this.assertGeneratedSubtree(filepath, resolved); return (await fs.readFile(resolved)).toString(); } public async put(filepath: string, data: string, basePathOverride?: string) { - const basePath = basePathOverride || this.outputPath; + const basePath = basePathOverride || this.baseFor(filepath); const resolved = assertWithinBase(filepath, basePath); + this.assertGeneratedSubtree(filepath, resolved); await fs.outputFile(resolved, data); } } diff --git a/packages/@tinacms/graphql/tests/filesystem-bridge/index.test.ts b/packages/@tinacms/graphql/tests/filesystem-bridge/index.test.ts index 8fe489326f..1d4b20acee 100644 --- a/packages/@tinacms/graphql/tests/filesystem-bridge/index.test.ts +++ b/packages/@tinacms/graphql/tests/filesystem-bridge/index.test.ts @@ -142,14 +142,141 @@ describe('filesystem bridge', () => { }); }); +describe('multi-repo routing (rootPath vs outputPath)', () => { + let generatorDir: string; + let contentDir: string; + + beforeEach(async () => { + const stamp = Date.now(); + generatorDir = path.join(os.tmpdir(), `tinacms-bridge-gen-${stamp}`); + contentDir = path.join(os.tmpdir(), `tinacms-bridge-content-${stamp}`); + await fs.mkdirp(generatorDir); + await fs.mkdirp(contentDir); + }); + + afterEach(async () => { + await fs.remove(generatorDir); + await fs.remove(contentDir); + }); + + test('put writes tina/__generated__/* to rootPath, never outputPath', async () => { + const bridge = new FilesystemBridge(generatorDir, contentDir); + await bridge.put('tina/__generated__/_schema.json', '{"x":1}'); + + expect( + await fs.pathExists( + path.join(generatorDir, 'tina/__generated__/_schema.json') + ) + ).toBe(true); + expect( + await fs.pathExists( + path.join(contentDir, 'tina/__generated__/_schema.json') + ) + ).toBe(false); + }); + + test('get reads tina/__generated__/* from rootPath', async () => { + const bridge = new FilesystemBridge(generatorDir, contentDir); + await fs.outputFile( + path.join(generatorDir, 'tina/__generated__/_graphql.json'), + '{"fromGenerator":true}' + ); + await fs.outputFile( + path.join(contentDir, 'tina/__generated__/_graphql.json'), + '{"fromContent":true}' + ); + + const result = await bridge.get('tina/__generated__/_graphql.json'); + expect(result).toBe('{"fromGenerator":true}'); + }); + + test('routes .tina/__generated__/* (legacy) to rootPath as well', async () => { + const bridge = new FilesystemBridge(generatorDir, contentDir); + await bridge.put('.tina/__generated__/_lookup.json', '{"k":"v"}'); + + expect( + await fs.pathExists( + path.join(generatorDir, '.tina/__generated__/_lookup.json') + ) + ).toBe(true); + }); + + test('content files still route to outputPath', async () => { + const bridge = new FilesystemBridge(generatorDir, contentDir); + await bridge.put('rules/example/index.md', 'body'); + + expect( + await fs.pathExists(path.join(contentDir, 'rules/example/index.md')) + ).toBe(true); + expect( + await fs.pathExists(path.join(generatorDir, 'rules/example/index.md')) + ).toBe(false); + }); + + test('delete respects the same routing', async () => { + const bridge = new FilesystemBridge(generatorDir, contentDir); + const genFile = path.join(generatorDir, 'tina/__generated__/_schema.json'); + const contentFile = path.join(contentDir, 'rules/doomed.md'); + await fs.outputFile(genFile, '{"x":1}'); + await fs.outputFile(contentFile, 'content'); + + await bridge.delete('tina/__generated__/_schema.json'); + await bridge.delete('rules/doomed.md'); + + expect(await fs.pathExists(genFile)).toBe(false); + expect(await fs.pathExists(contentFile)).toBe(false); + }); + + test('directory-only path (no trailing slash) routes to outputPath, not rootPath', async () => { + // Boundary case: isGeneratedPath uses startsWith('tina/__generated__/'). + // The exact directory string `tina/__generated__` (no trailing slash) + // does not match the prefix and therefore routes to outputPath. This is + // documented intentional — bridge callers always pass file paths, never + // bare directory names. Pinning the boundary here so a future regression + // (e.g. adding directory operations) gets caught. + const bridge = new FilesystemBridge(generatorDir, contentDir); + await fs.outputFile( + path.join(contentDir, 'tina/__generated__'), + 'sentinel' + ); + + const result = await bridge.get('tina/__generated__'); + expect(result).toBe('sentinel'); + }); + + test('assertWithinBase still rejects path traversal that escapes the routed base', async () => { + // The routing picks rootPath because the path starts with + // `tina/__generated__/`, but the trailing `..`s walk past rootPath itself + // — assertWithinBase must throw. This pins that the security check still + // runs, just against the chosen base. + const traversalPath = + 'tina/__generated__/../../../../../../../../etc/escape.txt'; + const bridge = new FilesystemBridge(generatorDir, contentDir); + await expect(bridge.put(traversalPath, 'pwned')).rejects.toThrow( + /traversal/i + ); + await expect(bridge.get(traversalPath)).rejects.toThrow(/traversal/i); + await expect(bridge.delete(traversalPath)).rejects.toThrow(/traversal/i); + }); + + test('rejects generated-prefixed paths that escape the generated subtree but stay in rootPath', async () => { + // Defense-in-depth: `tina/__generated__/../../.env` resolves to + // `/.env` — inside rootPath, but outside the generated subtree + // the routing is supposed to expose. The new assertGeneratedSubtree check + // catches this even though plain assertWithinBase would let it through. + const escapePath = 'tina/__generated__/../../.env'; + const bridge = new FilesystemBridge(generatorDir, contentDir); + await expect(bridge.put(escapePath, 'leak')).rejects.toThrow(/traversal/i); + await expect(bridge.get(escapePath)).rejects.toThrow(/traversal/i); + await expect(bridge.delete(escapePath)).rejects.toThrow(/traversal/i); + }); +}); + describe('AuditFileSystemBridge', () => { let tmpDir: string; beforeEach(async () => { - tmpDir = path.join( - process.env.TMPDIR || '/tmp', - `tinacms-audit-bridge-${Date.now()}` - ); + tmpDir = path.join(os.tmpdir(), `tinacms-audit-bridge-${Date.now()}`); await fs.mkdirp(tmpDir); }); diff --git a/packages/@tinacms/metrics/src/interfaces/index.ts b/packages/@tinacms/metrics/src/interfaces/index.ts index d9d3d89f80..c3d432835c 100644 --- a/packages/@tinacms/metrics/src/interfaces/index.ts +++ b/packages/@tinacms/metrics/src/interfaces/index.ts @@ -32,12 +32,25 @@ export interface TinaCMSServerError extends EventsBase { name: 'tinacms:cli:server:error'; errorMessage: string; } + +export interface TinaCMSDevInvoke extends EventsBase { + name: 'tinacms:cli:dev:invoke'; + hasLocalContentPath: boolean; +} + +export interface TinaCMSBuildInvoke extends EventsBase { + name: 'tinacms:cli:build:invoke'; + hasLocalContentPath: boolean; +} + export type Events = | CreateTinaAppInvoke | TinaCMSAuditInvoke | TinaCMSInitInvoke | TinaCMSServerStartInvoke - | TinaCMSServerError; + | TinaCMSServerError + | TinaCMSDevInvoke + | TinaCMSBuildInvoke; type Merge = { [K in keyof A]: K extends keyof B ? B[K] : A[K];