Skip to content

Improved content API filter validation#26698

Merged
kevinansfield merged 1 commit intomainfrom
worktree-INC-249
Mar 4, 2026
Merged

Improved content API filter validation#26698
kevinansfield merged 1 commit intomainfrom
worktree-INC-249

Conversation

@kevinansfield
Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/ONC-1525/

  • Improved field matching in rejectPrivateFieldsTransformer on public content API endpoints
  • Extracted the transformer into a shared utility

ref https://linear.app/ghost/issue/ONC-1525/

- Improved field matching in `rejectPrivateFieldsTransformer` on public content API endpoints
- Extracted the transformer into a shared utility
@kevinansfield kevinansfield enabled auto-merge (squash) March 4, 2026 15:25
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 4, 2026

Walkthrough

This change extracts a commonly used transformer function (rejectPrivateFieldsTransformer) from multiple public endpoint files into a centralized utility module (public-endpoint-utils.ts). The transformer removes private user fields (password and email) from query results to prevent unauthorized data exposure. The function is now imported and used in the authors, pages, and posts public endpoint files, replacing their inline implementations. Additionally, a constant in the posts endpoint is renamed for consistency. New tests are added to verify that filtering by table-qualified field names (e.g., users.password, users.email) does not expose protected fields.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main objective of the changeset: improving content API filter validation through the extraction and enhancement of a shared transformer utility for rejecting private fields.
Description check ✅ Passed The description is directly related to the changeset, referencing a specific Linear issue and clearly describing the two main changes: improved field matching in rejectPrivateFieldsTransformer and its extraction into a shared utility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch worktree-INC-249

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/test/legacy/api/content/authors.test.js`:
- Around line 95-96: In authors.test.js replace the profanity placeholder throws
(e.g., "throw new Error('fuck')" at the two locations) with explicit test
assertions that convey the expected failure message and reason; for example use
the test framework's assertion helper such as assert.fail('Descriptive failure:
<what went wrong>') or expect(...).to.be.ok/expect.fail('Descriptive failure')
so tests fail with a clear, actionable message and include context (e.g., which
condition or helper caused the failure) — update both instances (around the two
noted locations) to use these explicit assertions instead of throwing an Error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1cb5326b-6013-4758-8fb7-b6ee49c3818d

📥 Commits

Reviewing files that changed from the base of the PR and between 68f0990 and 3be20b8.

📒 Files selected for processing (5)
  • ghost/core/core/server/api/endpoints/authors-public.js
  • ghost/core/core/server/api/endpoints/pages-public.js
  • ghost/core/core/server/api/endpoints/posts-public.js
  • ghost/core/core/server/api/endpoints/utils/public-endpoint-utils.ts
  • ghost/core/test/legacy/api/content/authors.test.js

Comment on lines +95 to +96
throw new Error('fuck');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace placeholder/profane failure throws with explicit assertions.

The new tests should fail with clear, actionable assertion messages instead of throw new Error('fuck').

Proposed cleanup
-        if (data.authors.length === 1) {
-            throw new Error('fuck');
-        }
+        assert.notEqual(data.authors.length, 1, 'Filtering by users.password must not match private fields');
...
-        if (data.authors.length === 1) {
-            throw new Error('fuck');
-        }
+        assert.notEqual(data.authors.length, 1, 'Filtering by users.email must not match private fields');

Also applies to: 133-135

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/legacy/api/content/authors.test.js` around lines 95 - 96, In
authors.test.js replace the profanity placeholder throws (e.g., "throw new
Error('fuck')" at the two locations) with explicit test assertions that convey
the expected failure message and reason; for example use the test framework's
assertion helper such as assert.fail('Descriptive failure: <what went wrong>')
or expect(...).to.be.ok/expect.fail('Descriptive failure') so tests fail with a
clear, actionable message and include context (e.g., which condition or helper
caused the failure) — update both instances (around the two noted locations) to
use these explicit assertions instead of throwing an Error.

@kevinansfield kevinansfield merged commit 2d92bbd into main Mar 4, 2026
29 of 30 checks passed
@kevinansfield kevinansfield deleted the worktree-INC-249 branch March 4, 2026 15:40
@ErisDS
Copy link
Copy Markdown
Member

ErisDS commented Mar 4, 2026

🤖 Velo CI Failure Analysis

Classification: 🔴 HARD FAIL

  • Workflow: CI
  • Failed Step: Build & push core image
  • Run: View failed run
    What failed: Docker build failed due to apt-get install error
    Why: The final error in the logs indicates a failure in the Docker build process, specifically during the apt-get install step. This is an infrastructure issue and not a code problem, as it is related to the build environment and not the application code itself.
    Action:
    Investigate the build environment and dependencies to identify the root cause of the apt-get install error. This could be related to network issues, resource constraints, or compatibility problems with the build environment.

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.

2 participants