Added curated private site access codes#28029
Merged
Merged
Conversation
Contributor
WalkthroughThis PR updates the private-site access code generator to replace a placeholder format with a curated word-based format. The generator now selects a random word from a fixed 48-word list and appends a zero-padded 3-digit number, returning codes in Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ghost/core/test/unit/server/services/settings/private-site-access-code.test.js`:
- Around line 15-18: Restore a strict content assertion for the curated word
list in the test "uses a 48-word lowercase list with no duplicate entries":
after the existing shape checks for ACCESS_CODE_WORDS, add a strict
equality/deep-equality assertion (e.g. assert.deepStrictEqual or snapshot)
comparing ACCESS_CODE_WORDS to the exact 48-word array so any accidental
substitution is detected; reference the ACCESS_CODE_WORDS constant and the test
case name when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a289c00-9911-4f24-b4f6-457d694e6683
📒 Files selected for processing (1)
ghost/core/test/unit/server/services/settings/private-site-access-code.test.js
Comment on lines
+15
to
+18
| it('uses a 48-word lowercase list with no duplicate entries', function () { | ||
| assert.equal(ACCESS_CODE_WORDS.length, 48); | ||
| assert.equal(new Set(ACCESS_CODE_WORDS).size, ACCESS_CODE_WORDS.length); | ||
| assert.ok(ACCESS_CODE_WORDS.every(word => /^[a-z]+$/.test(word))); |
Contributor
There was a problem hiding this comment.
Reinstate an exact-content assertion for the curated word list.
At Line 15–18, the test now validates only shape (count/uniqueness/lowercase), which weakens protection for the Product-approved curated set. This will miss accidental word substitutions as long as they still match the regex and count. Please add back a strict equality/snapshot check for the exact 48 words.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ghost/core/test/unit/server/services/settings/private-site-access-code.test.js`
around lines 15 - 18, Restore a strict content assertion for the curated word
list in the test "uses a 48-word lowercase list with no duplicate entries":
after the existing shape checks for ACCESS_CODE_WORDS, add a strict
equality/deep-equality assertion (e.g. assert.deepStrictEqual or snapshot)
comparing ACCESS_CODE_WORDS to the exact 48-word array so any accidental
substitution is detected; reference the ACCESS_CODE_WORDS constant and the test
case name when making the change.
Member
Author
There was a problem hiding this comment.
What does this protect against? Duplicating the list in tests seems overkill
Contributor
There was a problem hiding this comment.
@ErisDS — That's a fair point, and I think you're right. The only scenario my suggestion would catch is someone swapping one curated word for another while keeping the count at 48 and all words lowercase — but that's an intentional edit to the source file, not an accidental regression. Duplicating the full array in the test is effectively copying a production constant, which adds maintenance burden without meaningfully improving safety. The existing shape-and-membership checks (format regex, word-is-in-list, length, uniqueness, lowercase) give good behavioral coverage. Happy to dismiss this one.
ᓚᘏᗢ
🧠 Learnings used
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/core/server/api/endpoints/member-comment-ban.js:64-68
Timestamp: 2026-01-08T10:26:38.700Z
Learning: In the Ghost API, endpoints rely on the serialization layer to prepare frame.data[docName] as a non-empty array before query() executes. Endpoints access frame.data[docName][0] directly (e.g., frame.data.comment_bans[0], frame.data.members[0], frame.data.posts[0]) without per-endpoint validation. This pattern is common across API endpoints. When maintaining or creating endpoints, avoid duplicating validation for frame.data[docName] and ensure the serializer guarantees the shape and non-emptiness. If you add a new endpoint that uses this frame.data[docName], follow the same assumption and avoid redundant checks unless there's a documented exception.
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/test/e2e-api/admin/member-commenting.test.js:268-321
Timestamp: 2026-01-26T13:53:03.597Z
Learning: In Ghost's e2e API tests, audit events should separate the generic operation from the specific action. Assert that event equals the CRUD operation type (e.g., 'edited', 'added', 'deleted') and, if needed, also assert on context.action_name for the exact action (e.g., 'commenting_disabled', 'commenting_enabled'). This makes tests robust to both high-level and specific audit details.
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 26219
File: ghost/core/test/e2e-api/members-comments/comments.test.js:939-983
Timestamp: 2026-02-04T15:58:09.124Z
Learning: In Ghost core tests and code that interact with the Ghost comments API, count.replies is a backward-compatible alias for count.total_replies (all descendants via parent_id) and does not represent direct replies. The new field count.direct_replies returns tree-native direct reply counts. Reviewers should verify any code paths, tests, or API surface areas that rely on count.replies are preserved for compatibility, and consider updating or adding tests to cover count.direct_replies for direct counts. When updating or adding tests, ensure behavior is documented and that any assertions reflect the distinction between total (including descendants) and direct reply counts to avoid regressions in API consumer expectations.
Learnt from: vershwal
Repo: TryGhost/Ghost PR: 26791
File: ghost/core/test/unit/server/services/media-inliner/test/external-media-inliner.test.js:1033-1039
Timestamp: 2026-03-12T10:43:01.366Z
Learning: In tests for the Ghost media-inliner, when reviews reference getMediaStorage withArgs(extension), verify that the extension corresponds to the actual binary content of the nock/mocked response (detected via magic bytes) rather than the URL filename. This applies to all tests under ghost/core/test/unit/server/services/media-inliner (and similar media-inliner tests). Use binary content to determine expected extension (e.g., a GIF blob should yield .gif even if the URL ends with .jpg).
Learnt from: vershwal
Repo: TryGhost/Ghost PR: 27290
File: ghost/core/package.json:76-77
Timestamp: 2026-04-09T09:44:26.783Z
Learning: In the TryGhost/Ghost monorepo, treat `tryghost/admin-api-schema` as the single abstraction layer over AJV version differences. Do not raise code review findings for AJV-internal error field changes (e.g., `dataPath` → `instancePath` between AJV v6 and v8) when evaluating Ghost consumer code. The consumer-facing error contract for this package (`ValidationError` with `message`, `property`, `errorDetails`) is expected to remain stable, and Ghost wrapper code should not inspect raw AJV error objects—so review should focus on the stable `ValidationError` shape rather than AJV internals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fake-###private-site access-code placeholder with the Product-approved curatedword###format.crypto.randomInt, choosing from the 48-word curated list plus a zero-padded three-digit suffix.Testing
source ~/.nvm/nvm.sh && nvm use 22 && pnpm test:single test/unit/server/services/settings/private-site-access-code.test.jssource ~/.nvm/nvm.sh && nvm use 22 && pnpm test:single test/unit/server/services/settings/settings-service.test.jssource ~/.nvm/nvm.sh && nvm use 22 && pnpm --filter @tryghost/parse-email-address install --offline && pnpm --filter @tryghost/parse-email-address build && pnpm test:single test/e2e-api/admin/settings.test.jssource ~/.nvm/nvm.sh && nvm use 22 && pnpm exec eslint core/server/services/settings/private-site-access-code.js test/unit/server/services/settings/private-site-access-code.test.js test/unit/server/services/settings/settings-service.test.js test/e2e-api/admin/settings.test.jsgit diff --check