Skip to content

Added publicSiteAccess limit enforcement to the Settings API#27917

Merged
ErisDS merged 2 commits into
mainfrom
add-public-site-access-readonly
May 15, 2026
Merged

Added publicSiteAccess limit enforcement to the Settings API#27917
ErisDS merged 2 commits into
mainfrom
add-public-site-access-readonly

Conversation

@ErisDS
Copy link
Copy Markdown
Member

@ErisDS ErisDS commented May 15, 2026

Summary

Adds API-layer enforcement for the new publicSiteAccess flag limit on @tryghost/limit-service. When Ghost(Pro) applies that limit to a trial site, the Settings BREAD service now:

  • marks is_private and password with is_read_only: true on browse — the field already flows through ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/settings.js:6 and Admin-X's isSettingReadOnly() helper, so this is just populating it server-side;
  • rejects external PUT /settings/ attempts to flip is_private = false or change the access code, returning NoPermissionError (403);
  • keeps internal-context edits open so boot-time enforcement and a future regenerate endpoint can still manage the values.

Why now, why dormant

The limit-service package was updated to recognise publicSiteAccess in TryGhost/SDK#918. The catalogued dependency in this repo is intentionally not bumped here — Renovate handles that as its own PR (see #27916, which isolates @tryghost/limit-service from the admin-support group so the bump lands standalone).

Until both the Renovate bump AND a Ghost(Pro) Daisy.js applied-limit land, limitsService.isDisabled('publicSiteAccess') returns undefined, the new code paths are no-ops, and nothing changes for any existing site.

The remaining trial-private-site backend work — boot-time persistence of is_private = true and the generated access-code helper — is deliberately scoped out and will follow in a separate PR.

Test plan

  • pnpm test:single test/unit/server/services/settings/settings-bread-service.test.js — 15 passing, including a new publicSiteAccess limit suite covering browse marking, edit rejection on is_private and password, no-op is_private = true edits, internal-context bypass, and the limit-off path.
  • pnpm test:single test/e2e-api/admin/settings.test.js — 28 passing. New cases sinon-stub limits.isDisabled('publicSiteAccess') on the singleton (with an inline comment explaining why) rather than driving the limit through configUtils.set + limits.init(), because this PR doesn't bump the SDK catalog version that recognises the limit name.
  • pnpm lint clean.

🤖 Generated with Claude Code

- gated on the `publicSiteAccess` flag limit from `@tryghost/limit-service`; when it is disabled (set by Ghost(Pro) for trial sites), the Settings BREAD service now marks `is_private` and `password` with `is_read_only: true` on browse, and rejects external attempts to flip `is_private = false` or change the access code on edit
- internal context still bypasses the rejection so boot-time enforcement and a future regenerate endpoint can manage the values
- the catalogued `@tryghost/limit-service` version is left untouched; Renovate will pull in the version that recognises `publicSiteAccess` separately, so this change is dormant in production until both that bump and the Daisy.js applied-limit land
- the boot-time persistence of `is_private = true` and the access-code generator are deliberately scoped out — they belong with the access-code generator follow-up

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 10912a7e-f757-4825-b935-aeae9fabe575

📥 Commits

Reviewing files that changed from the base of the PR and between 0e21bcd and 5ca09f6.

⛔ Files ignored due to path filters (1)
  • ghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • ghost/core/core/server/services/settings/settings-bread-service.js
  • ghost/core/test/unit/server/services/settings/settings-bread-service.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/test/unit/server/services/settings/settings-bread-service.test.js

Walkthrough

This PR adds enforcement of the publicSiteAccess limit to the SettingsBREADService. When the limitsService disables publicSiteAccess, the service prevents external requests from modifying is_private and password, marks those keys as is_read_only in browse responses via a new _isPublicSiteAccessLimited() helper, and throws a NoPermissionError with messages.publicSiteAccessLocked for blocked edits. Unit and e2e tests cover internal vs external contexts and both limited and non-limited states.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding enforcement for the publicSiteAccess limit to the Settings API, which is the primary objective of this changeset.
Description check ✅ Passed The description is thorough and directly related to the changeset, explaining the motivation, implementation details, dependency handling, and comprehensive test coverage for the publicSiteAccess limit enforcement.
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.

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

✨ 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 add-public-site-access-readonly

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

Avoids mentioning "trial" in the user-facing error since the
publicSiteAccess limit name is deliberately context-agnostic and may
apply to non-trial states in future.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ErisDS ErisDS merged commit 77c505d into main May 15, 2026
43 checks passed
@ErisDS ErisDS deleted the add-public-site-access-readonly branch May 15, 2026 13:14
ErisDS added a commit that referenced this pull request May 18, 2026
The Settings BREAD service (merged in #27917) prevents runtime API
mutations of is_private and the access code while the publicSiteAccess
limit is disabled — but nothing was making the site private in the
first place. Without a boot-time pass, visitors could see a public
site in the window between boot and the first API write.

This adds enforcePublicSiteAccessLimit() between populateDefaults()
and SettingsCache.init(). It reads the in-memory settings collection,
persists any missing values with internal context, and refetches before
the cache reads it. After the first boot the values already match, so
steady state is zero writes.

The access-code generator is a placeholder (fake-###) — the real
format needs a separate design pass and will land in a follow-up PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ErisDS added a commit that referenced this pull request May 18, 2026
The Settings BREAD service (merged in #27917) prevents runtime API
mutations of is_private and the access code while the publicSiteAccess
limit is disabled — but nothing was making the site private in the
first place. Without a boot-time pass, visitors could see a public
site in the window between boot and the first API write.

This adds enforcePublicSiteAccessLimit() between populateDefaults()
and SettingsCache.init(). It reads the in-memory settings collection,
persists any missing values with internal context, and refetches before
the cache reads it. After the first boot the values already match, so
steady state is zero writes.

The access-code generator is a placeholder (fake-###) — the real
format needs a separate design pass and will land in a follow-up PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ErisDS added a commit that referenced this pull request May 18, 2026
)

The BREAD service (#27917) prevents runtime mutations of is_private and 
the access code while the publicSiteAccess limit is active, but nothing
was making the site private in the first place. Without a boot-time
pass, visitors could see a public site in the gap between boot and the
first API write.

This runs before the settings cache is initialized, using findOne/edit
on the model directly so the cache sees the final state on first init.
After first boot the values already match, so subsequent boots are
zero-write no-ops.

The access-code generator is a placeholder (fake-###) - the real format
needs a separate design pass.
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.

1 participant