Skip to content

feat(plugins): render contentAccess tables in the install consent dialog - #336

Open
mostafasadeghidev wants to merge 3 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/plugin-consent-content-tables
Open

mostafasadeghidev wants to merge 3 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/plugin-consent-content-tables

Conversation

@mostafasadeghidev

Copy link
Copy Markdown
Contributor

What changed

The plugin install/upgrade consent dialog rendered requested permissions and networkAllowedHosts but silently omitted the manifest's contentAccess[] per-table allowlist — even though the SDK docs claimed the consent screen showed it. An operator could approve a plugin without ever seeing which content tables it may read or write. This adds a "Content tables" section mirroring the existing "External hosts" section.

  • New diff helper (computeContentAccessDiff.ts): diffs entries by table into new / existing / dropped rows on upgrade, with the same ordering and badges as the host diff. One deliberate extension: an already-approved table whose upgrade adds modes (e.g. read → read, write) is promoted to "new" and the row calls out the newly requested modes — a silent privilege expansion cannot hide behind an "Already approved" badge.
  • Rendering (PermissionReviewSection.tsx): each row shows the table slug in code style with its modes ("Read, write"). The @own-created marker renders human-readably as "Tables this plugin creates" instead of a literal slug — forward-compatible: marker rows appear only once the marker's parser change lands (separate PR).
  • Plumbing: usePluginsWorkspace threads previousContentAccess from the installed plugin's manifest, exactly like previousNetworkAllowedHosts.
  • Styling: the network-specific CSS classes became shared .allowlistSection / .allowlistHeader; dropped rows now strike through <code> labels too — which also fixes dropped host rows, which previously had no strikethrough. All tokens, CSS Modules only, no new colors.
  • Docs: consent-screen description in plugin-system.md, the contentAccess doc comment in the SDK manifest types, and the PLUGIN-001 e2e row.

Why

contentAccess is a security surface: it is the per-table allowlist the host enforces on every cms.content.* call. Consent that hides it asks the operator to approve access they cannot see.

Impact

Operators see exactly which tables a plugin wants, with per-mode diffing on upgrades. No behavior change for plugins; UI only.

Tests

9 new tests in permissionReviewSection.test.tsx: 5 unit tests for the differ (including the mode-expansion promotion) and 4 render tests (marker row, upgrade badge/callout path). 23/23 pass.

Verification

bun run build   # tsc -b && vite build — clean
bun run lint    # clean
bun test        # touched suites green

Remaining full-suite failures are pre-existing on this machine — verified by stashing the diff and rerunning: the failure list is byte-identical without these changes.

The install/upgrade consent dialog rendered requested permissions and
networkAllowedHosts but silently omitted the manifest's contentAccess[]
per-table allowlist, even though the SDK docs claimed the consent screen
showed it. Add a "Content tables" section mirroring the "External hosts"
section:

- computeContentAccessDiff diffs entries by table into new / existing /
  dropped rows on upgrade; a mode added to an already-approved table
  promotes the row to "new" and the row calls out the newly requested
  modes, so a silent privilege expansion (read -> read+write) cannot
  hide behind "Already approved".
- The @own-created marker renders human-readably as "Tables this plugin
  creates" instead of a literal slug.
- usePluginsWorkspace now threads previousContentAccess from the
  installed plugin's manifest, the same way it threads
  previousNetworkAllowedHosts.
- Shared allowlist-section styles (renamed from network-specific class
  names); dropped rows now strike through <code> labels too.
- Docs: plugin-system.md consent-screen description, the contentAccess
  doc comment in the SDK manifest types, and the PLUGIN-001 e2e row.

Verified with bun run build, bun run lint, and bun test on the touched
suites (permissionReviewSection, pluginsAdmin, architecture gates);
remaining suite failures reproduce identically without this diff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mostafasadeghidev
mostafasadeghidev marked this pull request as ready for review August 3, 2026 03:54
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Sep 5, 2026
…red migration

Two resolutions worth recording.

`manifest.ts` and `plugin-system.md` collided on the same doc text in two
versions. The stack's is the fuller one — it also describes the consent
screen CoreBunch#336 adds, which the CoreBunch#335 branch alone knows nothing about — so the
stack's copy wins both.

The migrations needed care in both directions. CoreBunch#335 renumbers this fork's
`025_data_tables_created_by_plugin` to `027` for upstream, which is correct
there and wrong here: two live installations have already recorded the `025`
id, and the ALTER carries no guard, so a second id would re-run it against an
existing column and fail the boot. The merge left both entries; the `027`
duplicate is dropped and the fork keeps its own id.

Checking that turned up a real loss. `026_plugin_media_sources` arrived
upstream in e7a27ca and the commit IS in this branch's history, but the
entry was not in either dialect file — an earlier conflict resolution here
dropped it while keeping the code that reads the table
(`server/repositories/pluginMediaSources.ts` queries `plugin_media_sources`).
A fresh install would have created that table nowhere. Restored from upstream
verbatim, after our 025 so both dialects keep the same order.

Verified: tsc, build and lint clean; media + plugins + settings + parity
292/292; db and plugin-handler suites 40/40, which is what actually runs the
migrations; fork-stack-capabilities 12/12.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The only run this branch ever had failed on a workflow-file problem in the
repository at the time, before a single job started — so the PR has carried
no check results since August. Merging main picks up the current workflow.
No conflicts; the permission review tests pass locally.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Sep 13, 2026
… and refuse a doubled ALTER

README: CoreBunch#359 and CoreBunch#498 move to the landed table, the count drops to ten,
and CoreBunch#507's row says what it does now — every page, on every branch.

The fork gate's own rule is that a merged PR's row is deleted, since the
capability then arrives with main. Nine merged rows had never been removed;
they go. CoreBunch#336 was never pinned at all; it is now.

And one new check, for the one merge mistake that takes a server down on
boot: no column may be added by two different migrations. The stack runs
CoreBunch#335's ALTER as `025_…` while upstream will carry it as `031_…`; keeping
both when CoreBunch#335 lands makes every recorded installation run it twice.
Verified by planting that exact `031` entry — the test names the collision
and fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…this one

CoreBunch#335 merged with its own wording for the contentAccess doc comment. Kept
its `@own-created` paragraph and its opening line, and put this PR's
consent-screen paragraph after it in place of CoreBunch#335's shorter "ships in the
manifest so the operator can review" sentence, which it supersedes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
… stays out

Upstream shipped CoreBunch#335 with its ALTER as `031_data_tables_created_by_plugin`.
This branch has run that ALTER as `025_…` on both live installations, so the
incoming 031 entry is dropped from both migration files — keeping it would
re-run the ALTER and fail the boot. The 025 note now also says what an
installation needs before moving to plain upstream (record 031 as applied).
The 032 backfill note names the final ids.

Resolved hunk by hunk:
- import.ts, SettingsModal.{tsx,module.css}: upstream's copy. Only CoreBunch#335 and
  CoreBunch#499 had touched them on this branch, and upstream carries both (the
  import moved into applySiteBundle with createdByPluginId; the keycap now
  uses Button's align/fullWidth). Checked: no other fork line was in them.
- manifest.ts, plugin-system.md: the stack's side inside the conflict hunks
  only — that text is CoreBunch#336's, still in review; the rest of upstream's
  changes to both files merged normally.
- fork gate: CoreBunch#335 and CoreBunch#499 rows removed, the migration note says 031.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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