Skip to content

Added post export permissions to the backup integration - #30138

Merged
mike182uk merged 1 commit into
mainfrom
mike-gva-907
Aug 20, 2026
Merged

Added post export permissions to the backup integration#30138
mike182uk merged 1 commit into
mainfrom
mike-gva-907

Conversation

@mike182uk

@mike182uk mike182uk commented Aug 20, 2026

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/GVA-907/

  • Ghost(Pro) archives are gaining a post-analytics.csv alongside the existing members.csv, fetched from GET /ghost/api/admin/posts/export/ with the ghost-backup integration key, and that endpoint is gated on browse post
  • There is no explicit export posts permission, so we use browse posts - exactly how browse member was added to this role in v5.121.0 to allow the members export
  • This grants no data the key could not already reach, in either half of that CSV. Post content: posts is in the exporter's TABLES_ALLOWLIST, so a plain GET /db/ already returns every post's title, html and lexical. The analytics counts: they derive from emails, email_recipients, members_click_events and members_feedback, none of which are in that allowlist - but exportContent accepts an include option validated against BACKUP_TABLES (db.js:45-56), which contains all of them, so GET /db/?include=emails,members_click_events works with today's key
  • So what the permission actually adds is a presentation: asking Ghost to join those tables into a CSV, rather than dumping them and joining them by hand
  • It does open /posts/ browse generally rather than only the export route, since permissions are per action type rather than per endpoint
  • Applied in fixtures and with a migration so both new and existing sites get the update, and mirrored into the test fixtures
  • Verified against a site created before this change, so fixtures never re-ran and only the migration could have applied it: the role gains post|browse, and the ghost-backup key then gets a 200 from /posts/export/?limit=all. A Zapier key is still refused on /db/

@github-actions github-actions Bot added the migration [pull request] Includes migration for review label Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

It looks like this PR contains a migration 👀
Here's the checklist for reviewing migrations:

General requirements

  • ⚠️ Tested performance on staging database servers, as performance on local machines is not comparable to a production environment
  • Satisfies idempotency requirement (both up() and down())
  • Does not reference models
  • Filename is in the correct format (and correctly ordered)
  • Targets the next minor version
  • All code paths have appropriate log messages
  • Uses the correct utils
  • Contains a minimal changeset
  • Does not mix DDL/DML operations

Schema changes

  • Both schema change and related migration have been implemented
  • For index changes: has been performance tested for large tables
  • For new tables/columns: fields use the appropriate predefined field lengths
  • For new tables/columns: field names follow the appropriate conventions
  • Does not drop a non-alpha table outside of a major version

Data changes

  • Mass updates/inserts are batched appropriately
  • Does not loop over large tables/datasets
  • Defends against missing or invalid data
  • For settings updates: follows the appropriate guidelines

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro

Run ID: 7bd3a8d1-f392-4c56-b067-6a444313daf8

📥 Commits

Reviewing files that changed from the base of the PR and between 5fa7d38 and 4c67d5d.

📒 Files selected for processing (1)
  • ghost/core/test/unit/server/data/schema/integrity.test.js

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (11)
  • GitHub Check: Acceptance tests (Node 22.23.1, mysql8)
  • GitHub Check: Lint
  • GitHub Check: Acceptance tests (Node 22.23.1, better-sqlite3)
  • GitHub Check: Legacy tests (Node 22.23.1, better-sqlite3)
  • GitHub Check: Legacy tests (Node 22.23.1, mysql8)
  • GitHub Check: Check app version bump
  • GitHub Check: Build E2E Public App Assets
  • GitHub Check: Build Admin
  • GitHub Check: Build Docker Images
  • GitHub Check: Unit tests (Node 22.23.1)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,cjs,mjs}

📄 CodeRabbit inference engine (Custom checks)

**/*.{js,jsx,cjs,mjs}: New files are TypeScript: Fail if the PR adds a new .js/.jsx/.cjs/.mjs source file, unless it is: a DB
migration (ghost/core/core/server/data/migrations/), under apps/ember-admin/,
a tool/config file, under scripts/ or docker/, or generated/vendored code.
Modifying pre-existing JS files never fails this check.

Files:

  • ghost/core/test/unit/server/data/schema/integrity.test.js

⚙️ CodeRabbit configuration file

**/*.{js,jsx,cjs,mjs}: New source files must be TypeScript: flag new JS files as a required change
unless exempt (DB migrations, apps/ember-admin/, tool/config files, scripts/,
docker/, generated code).
Never request conversion of pre-existing JS files. If the PR substantially
reworks one (rewritten logic or significant new functions — not renames or
small fixes), you may leave ONE optional, non-blocking note for the whole PR
that those files are cheap TS-conversion candidates; skip minor changes and
exempt areas.
If the PR adds or changes a runtime boundary (parsing HTTP input, JSON, config,
external responses), suggest validating it — ideally with TS + Zod.

Files:

  • ghost/core/test/unit/server/data/schema/integrity.test.js
**/*

📄 CodeRabbit inference engine (AGENTS.md)

Always use pnpm, never npm or Yarn.

Files:

  • ghost/core/test/unit/server/data/schema/integrity.test.js

⚙️ CodeRabbit configuration file

**/*: Prioritise concrete correctness, security, data-integrity, compatibility,
and regression risks. Explain the failure mode and point to the affected
code. Do not report formatting, naming, import ordering, type errors, or
other findings already owned by configured static tools or failing GitHub
checks. Do not request speculative abstractions, broad refactors, generic
documentation, or tests unrelated to changed behaviour. Treat nearby
AGENTS.md files and mapped codebase documentation as authoritative; do not
enforce proposals, plans, or historical guidance as current policy.

Files:

  • ghost/core/test/unit/server/data/schema/integrity.test.js
**/*{.,-}{test,spec}.{js,jsx,ts,tsx}

⚙️ CodeRabbit configuration file

**/*{.,-}{test,spec}.{js,jsx,ts,tsx}: Review whether tests prove changed behaviour, meaningful error/edge paths, and
externally observable contracts without coupling to implementation details.
Prefer the lowest useful test layer. Do not demand broad E2E coverage for
isolated logic or repeat test-run failures already visible in GitHub checks.

Files:

  • ghost/core/test/unit/server/data/schema/integrity.test.js
🔇 Additional comments (1)
ghost/core/test/unit/server/data/schema/integrity.test.js (1)

39-39: LGTM!


Walkthrough

The DB Backup Integration role now includes Browse posts permission in the schema and test fixtures. A 6.60 migration adds this permission to existing installations. Migration, fixture relation, and integrity tests reflect the updated fixtures. An admin API end-to-end test verifies post analytics CSV export through the trailing-slash endpoint, including response status, headers, and CSV columns.

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Type-Safe Boundaries ✅ Passed The diff adds static fixture data and a migration that passes literals to the existing addPermissionToRole helper; it adds no boundary read, unchecked type bypass, or duplicated schema type.
New Files Are Typescript ✅ Passed The PR adds one .js file, and it is a DB migration under ghost/core/core/server/data/migrations/. All other JavaScript changes are modifications to existing files.
Title check ✅ Passed The title clearly summarizes the primary change: adding post export permissions to the backup integration.
Description check ✅ Passed The description directly explains the export endpoint, permission change, fixture updates, migration, and verification.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mike-gva-907

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4c67d5d

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 59s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 25s View ↗
nx run ghost:test:e2e ✅ Succeeded 3m 4s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 30s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 22s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 24s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 27s View ↗
nx run @tryghost/admin:build ✅ Succeeded 8s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-08-20 13:29:37 UTC

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.54%. Comparing base (c10ff08) to head (4c67d5d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #30138      +/-   ##
==========================================
- Coverage   75.54%   75.54%   -0.01%     
==========================================
  Files        1635     1635              
  Lines      145877   145920      +43     
  Branches    18189    18202      +13     
==========================================
+ Hits       110197   110229      +32     
- Misses      34652    34689      +37     
+ Partials     1028     1002      -26     
Flag Coverage Δ
e2e-tests 77.39% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ref https://linear.app/ghost/issue/GVA-907/

- Ghost(Pro) archives are gaining a `post-analytics.csv` alongside the existing
  `members.csv`, fetched from `GET /ghost/api/admin/posts/export/` with the
  `ghost-backup` integration key, and that endpoint is gated on `browse post`
- There is no explicit export posts permission, so we use browse posts - exactly
  how `browse member` was added to this role in v5.121.0 to allow the members
  export
- The role already holds `db: all`, and `posts` is in the database exporter's
  `TABLES_ALLOWLIST`, so the key can already read every post in full through the
  db export; this grants no data it could not already reach, just a narrower
  capability through a different door
- It does open `/posts/` browse generally rather than only the export route,
  since permissions are per action type rather than per endpoint
- Applied in fixtures and with a migration so both new and existing sites get the
  update, and mirrored into the test fixtures
@mike182uk
mike182uk merged commit b0af358 into main Aug 20, 2026
53 checks passed
@mike182uk
mike182uk deleted the mike-gva-907 branch August 20, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration [pull request] Includes migration for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants