fix: allow all intended actions for site permissions + rearrange the permissions UI

backend/api/approvals.ts

@@ -176,7 +176,9 @@ async function routes(app: FastifyInstance) {
     '/sites/:siteId/approvals/rules',
     {
       config: {
-        permissions: ['read:sites', 'manage:sites']
+        // -> `read:sites` stood beside this one and matched nobody: it is not a permission the group
+        //    editor offers, and nothing validates a name that is not
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'List the approval rules of a site',

backend/api/groups.ts

@@ -52,11 +52,18 @@ async function routes(app: FastifyInstance) {
     '/',
     {
       config: {
-        // -> `manage:navigation` is here because a menu item can be limited to groups, so the
-        //    navigation editor has to be able to name them. It is safe to grant on this route and this
-        //    route only: the listing is `GroupCore`, which carries no permissions, no rules and no
-        //    members — reading one group in full, or its members, keeps needing `manage:groups`.
-        permissions: ['read:groups', 'manage:groups', 'manage:navigation']
+        /*
+          `manage:navigation` is here because a menu item can be limited to groups, and `manage:sites`
+          because an approval rule names the groups that may suggest an edit and the groups that
+          review one — both editors have to be able to name a group they cannot otherwise read, and
+          the approvals screen loads this alongside its rules, so without it the screen does not open
+          at all.
+
+          Safe to grant on this route and this route only: the listing is `GroupCore`, which carries
+          no permissions, no rules and no members — reading one group in full, or its members, keeps
+          needing `manage:groups`.
+        */
+        permissions: ['read:groups', 'manage:groups', 'manage:navigation', 'manage:sites']
       },
       schema: {
         summary: 'List all groups',

backend/api/locales.ts

@@ -42,7 +42,13 @@ async function routes(app: FastifyInstance) {
     '/fetch',
     {
       config: {
-        permissions: ['manage:system']
+        /*
+          Adding a locale to the installation is additive: it downloads strings nobody had and leaves
+          every site exactly as it was. Site administrators need it because a site can only be given a
+          locale that is installed, so gating it on `manage:system` left them naming a locale they
+          could not fetch. Renaming one is the opposite — see `/:code/aliases` below.
+        */
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Fetch the latest locales from the Wiki.js repository',
@@ -86,7 +92,8 @@ async function routes(app: FastifyInstance) {
     '/:code/install',
     {
       config: {
-        permissions: ['manage:system']
+        // -> Site-bound in effect, like `/fetch` above: adding a locale is what lets a site use it
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Download the strings of an available locale',
@@ -135,7 +142,8 @@ async function routes(app: FastifyInstance) {
     '/upload',
     {
       config: {
-        permissions: ['manage:system']
+        // -> Site-bound in effect, like `/fetch` above: adding a locale is what lets a site use it
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Install a locale from an uploaded strings file',
@@ -194,6 +202,9 @@ async function routes(app: FastifyInstance) {
     '/:code/aliases',
     {
       config: {
+        // -> Not `manage:sites`, unlike installing one above: a locale's name and short code are how
+        //    EVERY site on the instance refers to it and addresses it in a URL, so renaming one from
+        //    a single site's admin area would reach across all of them
         permissions: ['manage:system']
       },
       schema: {

backend/api/pages.ts

@@ -22,6 +22,20 @@ function splitList(value?: string): string[] {
   )
 }
 
+/**
+ * Whether two tag lists say the same thing.
+ *
+ * A set rather than an array comparison: a tag is on a page or it is not, so a list carrying the same
+ * tags in another order assigns nothing. It matters because the editor sends every field on every
+ * save — a body carrying the tags the page already has is not somebody retagging it, and treating it
+ * as one would need `write:tags` of anybody who ever saved a tagged page.
+ */
+function sameTags(a: string[], b: string[]): boolean {
+  const left = new Set(a)
+  const right = new Set(b)
+  return left.size === right.size && [...left].every((tag) => right.has(tag))
+}
+
 const siteIdParam = {
   type: 'object',
   properties: {
@@ -86,6 +100,7 @@ const PAGE_PERMISSIONS = [
   'review:pages',
   'manage:pages',
   'delete:pages',
+  'write:tags',
   'write:styles',
   'write:scripts',
   'read:source',
@@ -853,16 +868,23 @@ async function routes(app: FastifyInstance) {
         tags a new page carries are the ones in this request — and leaving them out would make a
         rule addressing tags silently miss every page the moment it was created.
       */
-      if (
-        !mayOnPage(req, 'write:pages', {
-          siteId: req.params.siteId,
-          path: req.body.path,
-          locale: req.body.locale,
-          tags: req.body.tags ?? []
-        })
-      ) {
+      const incoming = {
+        siteId: req.params.siteId,
+        path: req.body.path,
+        locale: req.body.locale,
+        tags: req.body.tags ?? []
+      }
+      if (!mayOnPage(req, 'write:pages', incoming)) {
         return reply.forbidden('You are not allowed to create a page here.')
       }
+      /*
+        Tags are a permission of their own, and creating a page carrying them is assigning them —
+        otherwise the way around `write:tags` would be to make a new page instead of tagging an old
+        one. Only a non-empty list asks anything: a page created untagged assigns nothing.
+      */
+      if (incoming.tags.length > 0 && !mayOnPage(req, 'write:tags', incoming)) {
+        return reply.forbidden('You are not allowed to assign tags to a page here.')
+      }
       const { page, versionId } = await WIKI.models.pages.createPage(
         req.params.siteId,
         req.body,
@@ -933,21 +955,36 @@ async function routes(app: FastifyInstance) {
         return reply.forbidden('You are not allowed to edit this page.')
       }
       /*
-        And against the tags the edit gives it, when it changes them. Retagging a page is what a move
-        is to a path: a rule may address pages by tag, so writing a page INTO a set of tags the writer
-        has no say over is the same hole as moving one into a branch they could not have created a
-        page in — and the check below is the tag half of the one the move route makes.
+        Whether this save actually retags the page, which is the only thing any of the checks below
+        are about. Compared against the row rather than read off the body being present, because the
+        editor sends every field on every save — see `sameTags`.
       */
-      if (
-        req.body.tags !== undefined &&
-        !mayOnPage(req, 'write:pages', {
+      if (req.body.tags !== undefined && !sameTags(req.body.tags, target.tags)) {
+        const retagged = {
           siteId: req.params.siteId,
           path: target.path,
           locale: target.locale,
           tags: req.body.tags
-        })
-      ) {
-        return reply.forbidden('You are not allowed to give this page those tags.')
+        }
+        /*
+          Assigning and unassigning tags is a permission of its own, and it is asked of the page on
+          both sides of the change the way the move route asks about both locations: a rule may
+          address pages by tag, so taking the tag that carries the rule off a page is as much a
+          retagging as putting one on. Whoever may edit a page is not therefore whoever may decide
+          which set of rules it falls under.
+        */
+        if (!mayOnPage(req, 'write:tags', target) || !mayOnPage(req, 'write:tags', retagged)) {
+          return reply.forbidden('You are not allowed to change the tags on this page.')
+        }
+        /*
+          And write access to the page as the tags leave it. Retagging a page is what a move is to a
+          path: writing a page INTO a set of tags the writer has no say over is the same hole as
+          moving one into a branch they could not have created a page in — and this is the tag half
+          of the check the move route makes.
+        */
+        if (!mayOnPage(req, 'write:pages', retagged)) {
+          return reply.forbidden('You are not allowed to give this page those tags.')
+        }
       }
       const change = await WIKI.models.pages.updatePage(
         req.params.siteId,

backend/api/schemas/page.ts

@@ -117,7 +117,9 @@ export async function registerSchemas(app: FastifyInstance): Promise<void> {
         type: 'array',
         items: {
           type: 'string'
-        }
+        },
+        description:
+          'Changing these requires the `write:tags` permission, on the page as it stands and as the tags leave it. Sending the tags the page already carries asks nothing, which is what lets somebody without it save a tagged page.'
       },
       allowBacklinks: { type: 'boolean' },
       allowComments: { type: 'boolean' },

backend/api/schemas/storage.ts

@@ -96,7 +96,7 @@ export async function registerSchemas(app: FastifyInstance): Promise<void> {
         type: 'object',
         additionalProperties: true,
         description:
-          'The module configuration, declared in its `definition.yml`: each entry carries a `type`, `title`, `hint`, `default` and the display hints the admin area renders a control from. A `readOnly` prop is shown but cannot be changed, and is silently kept at its stored value when written to.'
+          'The module configuration, declared in its `definition.yml`: each entry carries a `type`, `title`, `hint`, `default` and the display hints the admin area renders a control from. A `readOnly` prop is shown but cannot be changed, and is silently kept at its stored value when written to.\n\n`localPath` says that a prop holds a path on this server, and what may be put in it: `data` is confined to the wiki data directory, `system` is anywhere on the machine. Without `manage:system` a `system` path also reads back as `readOnly`, since there is no value such a caller could set.'
       },
       config: {
         type: 'object',
@@ -214,7 +214,7 @@ export async function registerSchemas(app: FastifyInstance): Promise<void> {
         type: 'object',
         additionalProperties: true,
         description:
-          'Values for the module props. Validated against what the module declares: an unknown key is dropped, a wrong type is refused, and a read-only prop keeps its stored value. A sensitive prop sent back as the mask it was read as keeps its stored value too; send a new value to replace the secret, or an empty string to remove it.'
+          'Values for the module props. Validated against what the module declares: an unknown key is dropped, a wrong type is refused, and a read-only prop keeps its stored value. A sensitive prop sent back as the mask it was read as keeps its stored value too; send a new value to replace the secret, or an empty string to remove it.\n\nA prop declaring `localPath` is refused rather than dropped when the caller may not point it where they asked: without `manage:system`, a `data` path must resolve inside the wiki data directory and a `system` path cannot be set at all. Sending back the value that is already stored is always accepted, so a caller who may not change a path can still save every other field of the target.'
       }
     }
   })

backend/api/sites.ts

@@ -31,7 +31,6 @@ const SITE_CONFIG_KEYS = [
   'features',
   'locales',
   'robots',
-  'theme',
   'uploads'
 ] as const
 
@@ -54,7 +53,14 @@ async function routes(app: FastifyInstance) {
     '/',
     {
       config: {
-        permissions: ['read:sites', 'access:admin']
+        /*
+          `manage:sites` as well as `access:admin`, because managing sites starts with seeing which
+          ones there are — the admin area's site selector is filled from here, and every site-bound
+          screen hangs off it. `read:sites` used to stand where `manage:sites` does and was never a
+          permission anybody could hold: nothing validates a name, and one that is not on the list
+          the group editor offers simply never matches.
+        */
+        permissions: ['manage:sites', 'access:admin']
       },
       schema: {
         summary: 'List all sites',
@@ -161,7 +167,8 @@ async function routes(app: FastifyInstance) {
     '/',
     {
       config: {
-        permissions: ['create:sites', 'manage:sites']
+        // -> `create:sites` stood beside this one and matched nobody; see the note on the listing above
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Create a new site',
@@ -291,7 +298,6 @@ async function routes(app: FastifyInstance) {
         showMenu?: boolean
       }
       robots?: Record<string, any>
-      theme?: Record<string, any>
       uploads?: Record<string, any>
     }
   }>(
@@ -302,6 +308,8 @@ async function routes(app: FastifyInstance) {
       },
       schema: {
         summary: 'Update a site',
+        description:
+          'Every site setting except its theme, which has a route of its own because `manage:theme` grants it without granting the rest of this — see `PUT /sites/{siteId}/theme`.',
         tags: ['Sites'],
         params: {
           type: 'object',
@@ -382,9 +390,6 @@ async function routes(app: FastifyInstance) {
             robots: {
               $ref: 'Site#/properties/robots'
             },
-            theme: {
-              $ref: 'Site#/properties/theme'
-            },
             uploads: {
               $ref: 'Site#/properties/uploads'
             }
@@ -530,6 +535,81 @@ async function routes(app: FastifyInstance) {
     }
   )
 
+  /**
+   * UPDATE SITE THEME
+   *
+   * Its own route rather than a section of `PUT /:siteId`, because it is its own permission:
+   * `manage:theme` is how a wiki hands somebody the look of a site without handing them its
+   * hostname, its locales, its authentication or its page defaults. Folding the theme into the
+   * general update would mean either refusing a theme manager outright — which is what happened
+   * before this existed, since the admin area offers them the screen — or granting them every
+   * other setting in the same body.
+   *
+   * There is no matching `GET`: a site's theme is public, served with the site itself to every
+   * reader that has to draw it, so `GET /sites/{siteIdorHostname}` already answers with it and a
+   * second copy behind a permission would say the same thing less usefully.
+   */
+  app.put<{ Params: { siteId: string }; Body: Record<string, any> }>(
+    '/:siteId/theme',
+    {
+      config: {
+        // -> `manage:sites` too: whoever administers the site holds everything in it, and this route
+        //    is the only way the theme is written now that the general update has given it up
+        permissions: ['manage:sites', 'manage:theme']
+      },
+      schema: {
+        summary: "Update a site's theme",
+        description:
+          "The site's appearance: its colors, fonts, layout choices and the raw CSS, head and body it injects into every page. Merged onto what is stored, so a partial body leaves the rest alone.\n\nThe three `inject*` fields are served into the document as written — that is what they are for — so this route is a trust boundary, and `manage:theme` is a permission to hand out on that understanding.",
+        tags: ['Sites'],
+        params: {
+          type: 'object',
+          properties: {
+            siteId: {
+              type: 'string',
+              format: 'uuid'
+            }
+          },
+          required: ['siteId']
+        },
+        body: { $ref: 'Site#/properties/theme' },
+        response: {
+          200: {
+            description: 'Site theme updated successfully',
+            type: 'object',
+            properties: {
+              ok: {
+                type: 'boolean'
+              },
+              message: {
+                type: 'string'
+              }
+            }
+          }
+        }
+      }
+    },
+    async (req, reply) => {
+      const site = await WIKI.models.sites.getSiteById({ id: req.params.siteId })
+      if (!site) {
+        return reply.notFound('Site does not exist.')
+      }
+      await WIKI.models.sites.updateSite(req.params.siteId, { config: { theme: req.body } })
+
+      // -> Which fields were set, never their values: `injectCSS` and friends are whole stylesheets
+      //    and scripts, and the audit log records what was touched rather than copying content into it
+      await audit(req, 'admin', 'updateSiteTheme', {
+        siteId: req.params.siteId,
+        changedFields: Object.keys(req.body)
+      })
+
+      return {
+        ok: true,
+        message: 'Site theme updated successfully.'
+      }
+    }
+  )
+
   /**
    * UPLOAD SITE IMAGE
    */

backend/api/storage.ts

@@ -1,9 +1,61 @@
 import { audit } from '../helpers/audit.ts'
-import { maskSensitiveProps } from '../helpers/common.ts'
+import { dataPathRoot, maskSensitiveProps } from '../helpers/common.ts'
 import { STORAGE_DIRECT_ACCESS_FALLBACKS, STORAGE_TARGET_STATUSES } from '../models/storage.ts'
 import type { FastifyInstance } from 'fastify'
+import type { ModuleProp } from '../helpers/common.ts'
 import type { StorageSiteConfigInput, StorageTargetInput } from '../models/storage.ts'
 
+/**
+ * A target's path props as they stand for THIS caller.
+ *
+ * `storage.checkLocalPath` is the rule and refuses on save; this is the same rule said in the form,
+ * so that a site administrator reads what they may enter instead of finding out by being refused.
+ * Nothing here is the enforcement, and a client that ignores all of it is still refused.
+ *
+ * A `system` path — the git binary to run, the private key to read — is the operator's to point, so
+ * without `manage:system` there is no value such a caller could put in the field at all. Marked
+ * `readOnly`, which already means exactly that to the admin area: it disables the control and leaves
+ * the prop out of what is sent back, so whatever is stored is kept.
+ *
+ * A `data` path stays editable, because such a caller CAN set one — anywhere inside the wiki's data
+ * directory, which is where the default already is. Only its hint changes, to say where.
+ *
+ * The values are untouched either way: a path is not a secret, and hiding where a site's content is
+ * kept from the person administering that site would help nobody.
+ */
+function describeLocalPaths(
+  props: Record<string, ModuleProp>,
+  unconfined: boolean
+): Record<string, ModuleProp> {
+  if (unconfined) {
+    return props
+  }
+  return Object.fromEntries(
+    Object.entries(props).map(([key, prop]) => {
+      if (prop.localPath === 'system') {
+        return [
+          key,
+          {
+            ...prop,
+            readOnly: true,
+            hint: `${prop.hint} Only a system administrator can change this, as it points somewhere on this server rather than inside this wiki.`.trim()
+          }
+        ]
+      }
+      if (prop.localPath === 'data') {
+        return [
+          key,
+          {
+            ...prop,
+            hint: `${prop.hint} Has to be inside the wiki's data directory (${dataPathRoot()}) unless a system administrator sets it.`.trim()
+          }
+        ]
+      }
+      return [key, prop]
+    })
+  )
+}
+
 /**
  * Storage API Routes
  */
@@ -15,7 +67,16 @@ async function routes(app: FastifyInstance) {
     '/sites/:siteId/storage',
     {
       config: {
-        permissions: ['manage:system']
+        /*
+          A site's storage configuration is a site's own setting — where this site's content is
+          written and which target it is served from — so it belongs to whoever manages sites, the
+          way its analytics, comments and blocks do. What a target holds is not instance-wide: the
+          modules are installed with the wiki and this only says which of them this site uses.
+
+          Credentials are not what makes it `manage:system` either, because they never come back:
+          every `sensitive` prop is masked on the way out, here and in `PUT` below.
+        */
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Get the storage configuration of a site',
@@ -77,6 +138,7 @@ async function routes(app: FastifyInstance) {
       if (!site) {
         return reply.notFound('Site does not exist.')
       }
+      const unconfined = WIKI.models.groups.holdsSystemPermission(req)
       const layout = WIKI.models.storage.pathLayoutFor(req.params.siteId)
       return {
         largeThreshold: WIKI.models.storage.largeThresholdFor(req.params.siteId),
@@ -94,6 +156,7 @@ async function routes(app: FastifyInstance) {
         */
         targets: (await WIKI.models.storage.getSiteTargets(req.params.siteId)).map((target) => ({
           ...target,
+          props: describeLocalPaths(target.props, unconfined),
           config: maskSensitiveProps(target.props, target.config)
         }))
       }
@@ -107,9 +170,6 @@ async function routes(app: FastifyInstance) {
     '/sites/:siteId/storage/status',
     {
       config: {
-        // -> Deliberately not `manage:system`, unlike the rest of this file: this answers a status
-        //    light in the admin sidebar, which anybody who can see the storage section at all needs,
-        //    and it carries none of the configuration that makes the rest of these privileged
         permissions: ['manage:sites']
       },
       schema: {
@@ -176,7 +236,8 @@ async function routes(app: FastifyInstance) {
     '/sites/:siteId/storage',
     {
       config: {
-        permissions: ['manage:system']
+        // -> The same site-bound setting the `GET` above answers with; see the note there
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Update the storage configuration of a site',
@@ -254,6 +315,13 @@ async function routes(app: FastifyInstance) {
         return reply.notFound('Site does not exist.')
       }
 
+      /*
+        Whether this caller may point a path prop anywhere on this server. `manage:sites` is enough to
+        configure a site's storage, but a path is a place on the operator's machine rather than a
+        setting of the site — see `storage.checkLocalPath`, which is where the rule is.
+      */
+      const unconfined = WIKI.models.groups.holdsSystemPermission(req)
+
       // -> Validated as a whole first: a partially applied storage configuration is worse than a
       //    refused one, since the admin area saves every target at once
       const invalidConfig = WIKI.models.storage.validateSiteConfig(req.body)
@@ -267,7 +335,7 @@ async function routes(app: FastifyInstance) {
         if (!target) {
           return reply.notFound(`Storage target ${patch.id} does not exist.`)
         }
-        const invalid = WIKI.models.storage.validateTarget(target, patch)
+        const invalid = WIKI.models.storage.validateTarget(target, patch, { unconfined })
         if (invalid) {
           return reply.badRequest(invalid)
         }
@@ -314,7 +382,9 @@ async function routes(app: FastifyInstance) {
     '/sites/:siteId/storage/targets/:targetId/actions/:action',
     {
       config: {
-        permissions: ['manage:system']
+        // -> An action moves this site's content between this site's targets, so it is the same
+        //    authority as configuring them
+        permissions: ['manage:sites']
       },
       schema: {
         summary: 'Run an action on a storage target',

backend/helpers/common.ts

@@ -3,6 +3,7 @@ import { startCase } from 'es-toolkit/string'
 import crypto from 'node:crypto'
 import mime from 'mime'
 import fs from 'node:fs'
+import path from 'node:path'
 import type { FastifyReply, FastifyRequest } from 'fastify'
 
 export interface Deferred<T = void> {
@@ -366,6 +367,23 @@ export function getTypeDefaultValue(type: string): string | number | boolean | u
  */
 export type ModulePropDeclaration = ModulePropDefinition | string
 
+/**
+ * What a prop holding a path ON THIS SERVER is allowed to point at.
+ *
+ * `data` is somewhere inside the wiki's own data directory. A site administrator may set one freely:
+ * that directory is already the wiki's to write, so aiming a site's content tree at another folder
+ * in it reaches nothing they did not have.
+ *
+ * `system` is a path anywhere on the machine — a binary to execute, a private key to read. That is
+ * the operator's business rather than a site's, so only `manage:system` may set one. Confining it to
+ * the data directory instead would be no use: git is not installed there.
+ *
+ * Absent on every prop that is not a local path at all, which includes the ones that look like one:
+ * an object store's key prefix and an SFTP base path name a place on somebody else's server, where
+ * this process has no reach of its own.
+ */
+export type ModuleLocalPathScope = 'data' | 'system'
+
 export interface ModulePropDefinition {
   type: string
   default?: unknown
@@ -376,6 +394,7 @@ export interface ModulePropDefinition {
   multiline?: boolean
   sensitive?: boolean
   readOnly?: boolean
+  localPath?: ModuleLocalPathScope
   icon?: string
   order?: number
   if?: unknown[]
@@ -393,6 +412,8 @@ export interface ModuleProp {
   sensitive: boolean
   /** Shown but not editable — the module declares something this server cannot currently change. */
   readOnly: boolean
+  /** Null unless this prop holds a path on this server. See `ModuleLocalPathScope`. */
+  localPath: ModuleLocalPathScope | null
   icon: string
   order: number
   if: unknown[]
@@ -423,6 +444,43 @@ export function isSensitiveMask(prop: ModuleProp, value: unknown): boolean {
   return prop.sensitive && value === SENSITIVE_MASK
 }
 
+/**
+ * A path prop's value as an absolute path on this server.
+ *
+ * Relative to the install directory, which is what every hint on these props promises and what the
+ * modules themselves resolve against.
+ */
+export function resolveLocalPath(value: string): string {
+  return path.resolve(WIKI.ROOTPATH, value)
+}
+
+/** The wiki's own data directory, absolute. */
+export function dataPathRoot(): string {
+  return path.resolve(WIKI.ROOTPATH, WIKI.config.dataPath)
+}
+
+/**
+ * Whether a path is the wiki's data directory, or something inside it.
+ *
+ * Compared as resolved paths through `path.relative` rather than as strings, because a prefix test
+ * would accept `/data/wiki-elsewhere` for a data directory of `/data/wiki` — the sibling whose name
+ * merely starts the same way. `..` in the result is what says the path climbs back out; an absolute
+ * result is what says it was never under it at all (a different drive on Windows).
+ *
+ * Symlinks are not resolved: the check is about what an administrator may WRITE in a settings field,
+ * and following links would need the path to exist, which the folder a target is about to create
+ * does not yet.
+ */
+export function isWithinDataPath(value: string): boolean {
+  const root = dataPathRoot()
+  const resolved = resolveLocalPath(value)
+  if (resolved === root) {
+    return true
+  }
+  const relative = path.relative(root, resolved)
+  return relative.length > 0 && !relative.startsWith('..') && !path.isAbsolute(relative)
+}
+
 /**
  * A module's stored config with every sensitive value replaced by the mask.
  *
@@ -464,6 +522,7 @@ export function parseModuleProps(
       multiline: def.multiline || false,
       sensitive: def.sensitive || false,
       readOnly: def.readOnly || false,
+      localPath: def.localPath ?? null,
       icon: def.icon || 'rename',
       order: def.order || 100,
       if: def.if ?? []

backend/locales/en.json

@@ -229,6 +229,7 @@
   "admin.audit.actions.updateSecurity": "Changed the security configuration",
   "admin.audit.actions.updateSite": "Updated a site",
   "admin.audit.actions.updateSiteImage": "Uploaded a site image",
+  "admin.audit.actions.updateSiteTheme": "Updated a site theme",
   "admin.audit.actions.updateStorage": "Updated the storage configuration",
   "admin.audit.actions.updateUser": "Updated a user",
   "admin.audit.actions.updateUserDefaults": "Changed the user defaults",
@@ -631,6 +632,9 @@
   "admin.groups.selectedLocales": "Any Locale | {n} locale only | {count} locales selected",
   "admin.groups.selectedSites": "Any Site | 1 site selected | {count} sites selected",
   "admin.groups.subtitle": "Manage user groups and permissions",
+  "admin.groups.systemPermission": "Full Access",
+  "admin.groups.systemPermissionWarn": "This grants access to everything on this wiki.",
+  "admin.groups.systemPermissionWarnHint": "A member of this group can read, change and delete anything on every site, including other administrators and this group itself. Every permission beside it stops mattering. Grant it only to the people who run this instance.",
   "admin.groups.title": "Groups",
   "admin.groups.unassignUser": "Unassign User",
   "admin.groups.unassignUserConfirm": "Are you sure you want to unassign {userName} from this group?",

backend/models/auditLog.ts

@@ -106,6 +106,7 @@ export const AUDIT_ACTIONS = {
     'retryJob',
     'createSite',
     'updateSite',
+    'updateSiteTheme',
     'deleteSite',
     'updateSiteImage',
     'deleteSiteImage',

backend/models/storage.ts

@@ -2,7 +2,14 @@ import fs from 'node:fs/promises'
 import path from 'node:path'
 import { load } from 'js-yaml'
 import { and, eq, inArray } from 'drizzle-orm'
-import { CustomError, isSensitiveMask, parseModuleProps } from '../helpers/common.ts'
+import {
+  CustomError,
+  dataPathRoot,
+  isSensitiveMask,
+  isWithinDataPath,
+  parseModuleProps,
+  resolveLocalPath
+} from '../helpers/common.ts'
 import { sites as sitesTable, storage as storageTable } from '../db/schema.ts'
 import type { ModuleProp } from '../helpers/common.ts'
 import type { AssetKind } from './assets.ts'
@@ -794,15 +801,76 @@ class Storage {
     return config
   }
 
+  /**
+   * Whether this caller may set a path prop to this value.
+   *
+   * Only reached for a caller who does NOT hold `manage:system`, i.e. a site administrator. Three
+   * things follow from the fact that a path on this server is the operator's territory rather than a
+   * site's, and only the first is about where the path points:
+   *
+   * - **An unchanged value is always allowed.** A `manage:system` operator may point a target
+   *   wherever they like, and a site administrator saving any other field on that target sends the
+   *   whole configuration back — so treating a value they did not touch as a change would lock them
+   *   out of the rest of the form. The comparison is between RESOLVED paths, so re-spelling
+   *   `./data/content` as `data/content` is correctly not a change.
+   * - **A `data` path must land inside the data directory.** That directory is the wiki's own, so
+   *   there is nothing there a site administrator did not already have. Moving a target from an
+   *   outside path INTO it is therefore allowed, and is the one way they can change such a value:
+   *   it gives reach up, never out.
+   * - **A `system` path cannot be set at all.** `gitBinaryPath` is an executable this server runs
+   *   and `sshPrivateKeyPath` is a key it reads; neither has a sensible value inside the data
+   *   directory, so there is no confined form of them to offer. Keeping the stored value is all a
+   *   site administrator can do.
+   *
+   * @param stored The value as it currently stands, absent on a target being created
+   * @returns The reason it is refused, or null when it is allowed
+   */
+  checkLocalPath(prop: ModuleProp, value: unknown, stored: unknown): string | null {
+    if (typeof value !== 'string') {
+      // -> Left to the type check below, which words it for the prop rather than for the path
+      return null
+    }
+    const unchanged =
+      typeof stored === 'string' &&
+      (value === stored ||
+        // -> Both empty, both meaning "not set": neither resolves to a path to compare
+        (value.length > 0 &&
+          stored.length > 0 &&
+          resolveLocalPath(value) === resolveLocalPath(stored)))
+    if (unchanged) {
+      return null
+    }
+    // -> An empty value is a path being REMOVED rather than set — `gitBinaryPath` falls back to the
+    //    one on PATH, and nothing is read where a key path is blank
+    if (value.length < 1) {
+      return null
+    }
+    if (prop.localPath === 'system') {
+      return `Only a system administrator can set ${prop.title}, as it points somewhere on this server rather than inside this wiki.`
+    }
+    if (!isWithinDataPath(value)) {
+      return `${prop.title} has to be inside the wiki's data directory (${dataPathRoot()}). Only a system administrator can point it somewhere else.`
+    }
+    return null
+  }
+
   /**
    * Check incoming config values against what the module declares.
    *
    * The props are a runtime declaration read from a YAML file, so no JSON Schema can cover them —
    * without this, a boolean prop would happily store the string `"maybe"`.
    *
+   * @param options.stored The config as it currently stands, which is what a path prop is compared
+   *   against to decide whether this request is CHANGING it. Omit on a create, where there is none.
+   * @param options.unconfined Whether the caller may point a path prop anywhere on the machine, i.e.
+   *   whether they hold `manage:system`. See `checkLocalPath`.
    * @returns The reason it is invalid, or null when it is fine
    */
-  validateConfig(moduleKey: string, incoming: Record<string, any> = {}): string | null {
+  validateConfig(
+    moduleKey: string,
+    incoming: Record<string, any> = {},
+    options: { stored?: Record<string, any>; unconfined?: boolean } = {}
+  ): string | null {
     const props = this.getDefinition(moduleKey)?.props ?? {}
     for (const [key, value] of Object.entries(incoming)) {
       const prop = props[key]
@@ -812,6 +880,12 @@ class Storage {
       if (!prop || prop.readOnly || value === undefined || isSensitiveMask(prop, value)) {
         continue
       }
+      if (prop.localPath && !options.unconfined) {
+        const refusal = this.checkLocalPath(prop, value, options.stored?.[key])
+        if (refusal) {
+          return refusal
+        }
+      }
       if (prop.enum) {
         // -> Enum entries are declared as `value` or `value|label`
         const allowed = prop.enum.map((entry) => entry.split('|')[0])
@@ -843,9 +917,15 @@ class Storage {
   /**
    * Check a target patch against what its module supports.
    *
+   * @param options.unconfined Whether the caller may point a path prop anywhere on this server, i.e.
+   *   whether they hold `manage:system`. See `checkLocalPath`.
    * @returns The reason it is invalid, or null when it is fine
    */
-  validateTarget(target: StorageTarget, patch: StorageTargetInput): string | null {
+  validateTarget(
+    target: StorageTarget,
+    patch: StorageTargetInput,
+    options: { unconfined?: boolean } = {}
+  ): string | null {
     const definition = this.getDefinition(target.module)!
     if (patch.isEnabled === false && target.module === DB_MODULE) {
       return 'The database storage target cannot be disabled, as content would have nowhere to live.'
@@ -915,7 +995,10 @@ class Storage {
         return 'The Custom Base URL must be an http or https address.'
       }
     }
-    return this.validateConfig(target.module, patch.config)
+    return this.validateConfig(target.module, patch.config, {
+      stored: target.config,
+      unconfined: options.unconfined
+    })
   }
 
   /**

backend/modules/storage/disk/definition.yml

@@ -15,6 +15,7 @@ props:
     icon: symlink-directory
     order: 1
     default: ./data/content
+    localPath: data
 actions:
   exportAll:
     label: Export Everything

backend/modules/storage/git/definition.yml

@@ -63,6 +63,7 @@ props:
     hint: Absolute path to the key. The key must NOT be passphrase-protected.
     icon: key
     order: 12
+    localPath: system
     if:
       - { key: 'authType', eq: 'ssh' }
       - { key: 'sshPrivateKeyMode', eq: 'path' }
@@ -129,6 +130,7 @@ props:
     hint: Where the working copy is kept. Give each site its own path unless you turn on Add Site ID Prefix under Configuration, since two sites sharing a repository would otherwise write over each other. Relative paths are resolved from the Wiki.js install directory.
     icon: symlink-directory
     order: 32
+    localPath: data
   gitBinaryPath:
     type: String
     title: Git Binary Path
@@ -136,6 +138,7 @@ props:
     hint: Optional - Absolute path to the Git binary, when not available in PATH. Leave empty to use the default PATH location (recommended).
     icon: run-command
     order: 50
+    localPath: system
 actions:
   sync:
     label: Force Sync

frontend/src/components/GroupEditOverlay.vue

@@ -191,7 +191,7 @@
             flat
             color="grey"
             type="a"
-            :href="siteStore.docsBase + `/admin/permissions#rules`"
+            :href="siteStore.docsBase + `/admin/permissions#page-rules`"
             target="_blank" />
           <w-btn
             class="acrylic-btn mr-2"
@@ -470,7 +470,7 @@
                       flat
                       color="grey"
                       type="a"
-                      :href="siteStore.docsBase + `/admin/permissions#system-permissions`"
+                      :href="siteStore.docsBase + `/admin/permissions#global-permissions`"
                       target="_blank" />
                   </template>
                 </w-card-header>
@@ -498,6 +498,55 @@
                 </template>
               </w-card>
             </div>
+            <!--
+              `manage:system` on a card of its own, because it is not one more thing a group may do:
+              the server checks it FIRST and lets the request through whatever the list on the left
+              says, so ticking it makes every toggle beside it moot. A row at the bottom of that list
+              would have read as the tenth of ten.
+            -->
+            <div class="col-span-12 lg:col-span-6">
+              <w-card class="shadow-1 pb-2">
+                <w-card-header>{{ t(`admin.groups.systemPermission`) }}</w-card-header>
+                <w-item tag="label">
+                  <w-item-section class="items-center" style="flex: 0 0 40px">
+                    <w-icon name="la:snowflake" color="negative" size="sm" />
+                  </w-item-section>
+                  <w-item-section>
+                    <w-item-label>{{ systemPermission.permission }}</w-item-label>
+                    <w-item-label caption>{{ systemPermission.hint }}</w-item-label>
+                  </w-item-section>
+                  <w-item-section avatar>
+                    <!--
+                      No `color` or glyph overrides: WToggle says in its own header that the switch
+                      reads the same everywhere and that a toggle needing to signal danger should say
+                      so in its label. The card, the red mark beside it and the banner below are what
+                      say so here.
+                    -->
+                    <w-toggle
+                      v-model="state.group.permissions"
+                      :val="systemPermission.permission"
+                      :disable="isSystemPermissionLocked(systemPermission.permission)"
+                      :aria-label="systemPermission.permission" />
+                  </w-item-section>
+                </w-item>
+                <w-card-section>
+                  <!--
+                      A pale red panel on a light card and a TINT of the same red on a dark one, the
+                      way the dark surfaces elsewhere in the app are built: filling it solid read as
+                      an error that had happened rather than a caution about what is being granted.
+                    -->
+                  <w-banner class="bg-red-1 text-red-9 dark:bg-red-9/30 dark:text-red-2" dense>
+                    <template #avatar>
+                      <w-icon name="la:exclamation-triangle" color="negative" size="sm" />
+                    </template>
+                    <div class="font-semibold">{{ t('admin.groups.systemPermissionWarn') }}</div>
+                    <div class="mt-1 text-xs opacity-90">
+                      {{ t('admin.groups.systemPermissionWarnHint') }}
+                    </div>
+                  </w-banner>
+                </w-card-section>
+              </w-card>
+            </div>
           </div>
         </div>
       </w-page>
@@ -744,86 +793,69 @@ const usersHeaders = [
   }
 ]
 
+/**
+ * The group-wide permissions, in the order the screen offers them.
+ *
+ * Grouped by what they are about rather than alphabetically: getting into the admin area, then the
+ * site-bound screens, then the people screens, then the two read-only views. `manage:system` is
+ * deliberately NOT here — it is not one more entry on this list but the absence of the list, so it
+ * has a card of its own. See `systemPermission`.
+ */
 const permissions = [
   {
     permission: 'access:admin',
-    hint: 'Can access the administration area.',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
-  },
-  {
-    permission: 'read:users',
-    hint: 'Can view users, but not create or modify them.',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    hint: 'Can access the administration and view the dashboard. Cannot perform any other action unless other permissions are also granted.'
   },
   {
-    permission: 'manage:users',
-    hint: 'Can create / manage users (but not users with manage:system permissions)',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'manage:sites',
+    hint: 'Can create / manage sites, and every setting bound to one: general, analytics, approvals, comments, content blocks, editors, locale, login, storage and theme.'
   },
   {
-    permission: 'read:groups',
-    hint: 'Can view groups and their permissions, but not create or modify them.',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'manage:theme',
+    hint: 'Can modify site theme settings, including the CSS, head and body injected into every page.'
   },
   {
-    permission: 'manage:groups',
-    hint: 'Can create / manage groups and assign permissions (but not manage:system) / page rules',
-    warning: true,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'manage:navigation',
+    hint: 'Can manage site navigation'
   },
   {
-    permission: 'read:audit',
-    hint: 'Can read the audit log, i.e. the record of what everybody on this wiki has done.',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'read:users',
+    hint: 'Can view users, but not create or modify them.'
   },
   {
-    permission: 'read:metrics',
-    hint: 'Can scrape the Prometheus metrics endpoint from an address it is not open to anonymously.',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'manage:users',
+    hint: 'Can create / manage users (but not users with manage:system permissions)'
   },
   {
-    permission: 'manage:navigation',
-    hint: 'Can manage site navigation',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'read:groups',
+    hint: 'Can view groups and their permissions, but not create or modify them.'
   },
   {
-    permission: 'manage:theme',
-    hint: 'Can modify site theme settings',
-    warning: false,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'manage:groups',
+    hint: 'Can create / manage groups and assign permissions (but not manage:system) / page rules'
   },
   {
-    permission: 'manage:sites',
-    hint: 'Can create / manage sites',
-    warning: true,
-    restrictedForSystem: true,
-    disabled: false
+    permission: 'read:audit',
+    hint: 'Can read the audit log, i.e. the record of what everybody on this wiki has done.'
   },
   {
-    permission: 'manage:system',
-    hint: 'Can manage and access everything. Root administrator.',
-    warning: true,
-    restrictedForSystem: true,
-    disabled: true
+    permission: 'read:metrics',
+    hint: 'Can scrape the Prometheus metrics endpoint from an address it is not open to anonymously.'
   }
 ]
 
+/**
+ * The one permission that is not a permission to do something in particular.
+ *
+ * `manage:system` bypasses every check on the server rather than adding to what is granted, so a
+ * group holding it holds everything above whether or not any of it is ticked. Offered on a card of
+ * its own so that it cannot be read as the tenth item of a list of ten.
+ */
+const systemPermission = {
+  permission: 'manage:system',
+  hint: 'Can manage and access everything. Root administrator.'
+}
+
 /**
  * The subset of `rules` below that the guests group may be granted. Mirrors `GUEST_ROLES` in
  * `models/groups.ts`, which is the copy that decides — this one only shapes what is offered.
@@ -878,6 +910,14 @@ const rules = [
     restrictedForSystem: true,
     disabled: false
   },
+  {
+    permission: 'write:tags',
+    title: 'Assign Tags',
+    hint: 'Can assign and unassign tags on pages.',
+    warning: false,
+    restrictedForSystem: true,
+    disabled: false
+  },
   {
     permission: 'write:styles',
     title: 'Use CSS',

frontend/src/components/PagePropertiesDialog.vue

@@ -307,9 +307,14 @@
           </div>
         </w-form>
       </w-card-section>
-      <w-card-section class="pb-6" id="refCardTags">
+      <!--
+        Shown read-only to an author who may not retag the page, and left out altogether when there is
+        nothing to read either: a heading over an empty box says a page has no tags in a place that
+        cannot be used to give it any.
+      -->
+      <w-card-section class="pb-6" id="refCardTags" v-if="showTagsSection">
         <div class="w-section-header">{{ t('editor.props.tags') }}</div>
-        <page-tags edit />
+        <page-tags :edit="mayWriteTags" />
       </w-card-section>
       <w-card-section class="alt-card pb-6" id="refCardVisibility">
         <div class="w-section-header">{{ t('editor.props.visibility') }}</div>
@@ -447,6 +452,16 @@ const iptPagePassword = ref(null)
 const mayWriteScripts = computed(() => userStore.pagePermissions.includes('write:scripts'))
 const mayWriteStyles = computed(() => userStore.pagePermissions.includes('write:styles'))
 
+/*
+  And whether they may retag it, which the same rules answer separately: writing a page and deciding
+  which tags — and so which rules — it falls under are two permissions. The field is what assigns one,
+  so without it the section reads the tags out and offers no way to change them, exactly as the
+  PATCH route would refuse a body that did.
+*/
+const mayWriteTags = computed(() => userStore.pagePermissions.includes('write:tags'))
+// -> Named apart from `pageStore.showTags`, which is the page's own choice to display them
+const showTagsSection = computed(() => mayWriteTags.value || pageStore.tags?.length > 0)
+
 /*
   The rail of jump links down the side of the panel. A computed rather than a constant because the
   Scripts section is not always there, and a link to a section that is not rendered is a link that
@@ -465,7 +480,12 @@ const quickaccess = computed(() =>
     },
     { key: 'refCardSidebar', icon: 'la:ruler-vertical', label: t('editor.props.sidebar') },
     { key: 'refCardSocial', icon: 'la:comments', label: t('editor.props.social') },
-    { key: 'refCardTags', icon: 'la:tags', label: t('editor.props.tags') },
+    {
+      key: 'refCardTags',
+      icon: 'la:tags',
+      label: t('editor.props.tags'),
+      shown: showTagsSection.value
+    },
     { key: 'refCardVisibility', icon: 'la:eye', label: t('editor.props.visibility') }
   ].filter((qa) => qa.shown !== false)
 )

frontend/src/layouts/AdminLayout.vue

@@ -124,7 +124,8 @@
             </w-item>
             <w-item
               :to="`/_admin/` + adminStore.currentSiteId + `/general`"
-              active-class="bg-primary text-white">
+              active-class="bg-primary text-white"
+              v-if="userStore.can(`manage:sites`)">
               <w-item-section avatar>
                 <w-icon name="img:/_assets/icons/fluent-web.svg" />
               </w-item-section>
@@ -141,7 +142,8 @@
             </w-item>
             <w-item
               :to="`/_admin/` + adminStore.currentSiteId + `/approvals`"
-              active-class="bg-primary text-white">
+              active-class="bg-primary text-white"
+              v-if="userStore.can(`manage:sites`)">
               <w-item-section avatar>
                 <w-icon name="img:/_assets/icons/fluent-inspection.svg" />
               </w-item-section>

frontend/src/pages/AdminDashboard.vue

@@ -153,11 +153,18 @@
           </w-card-section>
           <w-separator />
           <w-card-actions align="right">
+            <!--
+              Guarded like every other action on this dashboard, and on the permission the Analytics
+              screen itself needs. Without this the one card action with no guard handed a live link
+              into a section `access:admin` alone cannot load -- every sibling button was already
+              disabled, so it read as the one thing such a user was meant to be able to open.
+            -->
             <w-btn
               flat
               :color="actionColor"
               icon="la:chart-area"
               :label="t(`admin.analytics.title`)"
+              :disable="!userStore.can(`manage:sites`)"
               :to="`/_admin/` + adminStore.currentSiteId + `/analytics`" />
           </w-card-actions>
         </w-card>

frontend/src/pages/AdminLocale.vue

@@ -133,7 +133,12 @@
                 <w-item-label>{{ lc.name }}</w-item-label>
                 <w-item-label caption>{{ lc.nativeName }} ({{ lc.displayCode }})</w-item-label>
               </w-item-section>
-              <w-item-section v-if="lc.isInstalled" side>
+              <!--
+                Installing a locale is a site administrator's to do, but renaming one is not: the name
+                and the short code are how every site on the instance addresses it, so the endpoint
+                behind this button asks for `manage:system` and the button is offered to nobody else.
+              -->
+              <w-item-section v-if="lc.isInstalled && userStore.can(`manage:system`)" side>
                 <w-btn
                   flat
                   dense
@@ -198,6 +203,7 @@ import { apiErrorMessage } from '@/helpers/apiError'
 
 import { useAdminStore } from '@/stores/admin'
 import { useSiteStore } from '@/stores/site'
+import { useUserStore } from '@/stores/user'
 
 import { sortBy } from 'es-toolkit/array'
 
@@ -209,6 +215,7 @@ const dark = useDark()
 
 const adminStore = useAdminStore()
 const siteStore = useSiteStore()
+const userStore = useUserStore()
 
 // I18N
 

frontend/src/pages/AdminTheme.vue

@@ -733,10 +733,13 @@ async function save() {
       baseFont: state.config.baseFont,
       contentFont: state.config.contentFont
     }
-    const resp = await API_CLIENT.put(`sites/${adminStore.currentSiteId}`, {
-      json: {
-        theme: patchTheme
-      }
+    /*
+      The theme's own endpoint rather than the general site update, because it is its own permission:
+      `manage:theme` grants the look of a site without granting its hostname, locales or
+      authentication, and the general update asks for `manage:sites`.
+    */
+    const resp = await API_CLIENT.put(`sites/${adminStore.currentSiteId}/theme`, {
+      json: patchTheme
     }).json()
     if (!resp?.ok) {
       throw new Error(

frontend/src/pages/Index.vue

@@ -323,7 +323,7 @@
                 for them it is not a control that happens to be out of sight.
               -->
               <w-btn
-                v-if="canEditPage"
+                v-if="canEditTags"
                 class="tags-edit-btn"
                 :class="{ 'is-hidden': !state.tagEditMode && !state.showTagsEditBtn }"
                 size="sm"
@@ -604,9 +604,9 @@ const showTags = computed(() => {
   return pageStore.showTags && (pageStore.tags?.length > 0 || state.tagEditMode)
 })
 /*
-  Whether this user may save a change to the page, which is what editing the tags amounts to -- the tags
-  go up with the rest of the page rather than through an endpoint of their own. So the test is the pair
-  the PATCH route accepts: `write:pages` or `manage:pages`.
+  Whether this user may save a change to the page. Editing the tags is a save -- they go up with the
+  rest of the page rather than through an endpoint of their own -- so the test is the pair the PATCH
+  route accepts: `write:pages` or `manage:pages`.
 
   Read off `pagePermissions` rather than through `userStore.can()`, which asks a broader question: the
   group-wide list from `whoami` says what a user may do somewhere, and the rules decide where. What
@@ -619,6 +619,16 @@ const canEditPage = computed(() =>
   )
 )
 
+/*
+  Whether this user may edit the TAGS, which takes one permission more than saving the page does.
+  Assigning a tag is what decides which rules a page falls under, so it is granted apart from writing
+  the page -- and both are needed here, since the tags travel up with the page and the PATCH route
+  asks for both in turn.
+*/
+const canEditTags = computed(
+  () => canEditPage.value && userStore.pagePermissions.includes('write:tags')
+)
+
 /*
   Whether the missing-page screen offers to create the page. `write:pages` at THIS path, from the same
   list as the tag button above: page rules are written against paths, not against pages, so they answer