Split dashboard.view/dashboard.manage permissions; harden query guard

README.md

@@ -143,13 +143,18 @@ the authoritative implementation:
    `COPY`/`CALL`/`EXPLAIN`/`VACUUM`, and nothing after a trailing `;`.
 2. No `information_schema`/`pg_catalog`/`pg_*` introspection, `dblink`,
    `lo_*`, or `COPY` — closes standard sandbox-escape tricks.
-3. **Project/integration-scoped queries must contain the literal
-   placeholder `{{project_id}}`** somewhere in a `WHERE`/`ON`/`HAVING`
-   clause. The guard substitutes it with `$1` and binds the caller's real
+3. **Project/integration-scoped queries must use the literal placeholder
+   `{{project_id}}` exactly once, as a direct `<column> = {{project_id}}`
+   (or reversed) equality filter, with no bare `OR` anywhere in the
+   query.** The guard substitutes it with `$1` and binds the caller's real
    `project_id` — this is what actually prevents cross-project data
-   leakage, by forcing every project-scoped query to filter itself.
-   Admin-scope queries are intentionally cross-project and skip this
-   requirement (and reject the placeholder if present).
+   leakage, by forcing every project-scoped query to filter itself in a way
+   that can't be short-circuited (a duplicated/self-compared placeholder, a
+   comparison operator other than `=`, or a sibling `OR` can all make the
+   filter match regardless of project — see `validateProjectScopePlaceholder`
+   in `query_guard.go`). Need a multi-value filter? Use `IN (...)` instead
+   of `OR`. Admin-scope queries are intentionally cross-project and skip
+   this requirement (and reject the placeholder if present).
 4. An explicit `LIMIT 500` is appended when the query doesn't already
    declare one.
 
@@ -179,6 +184,78 @@ rather than risk a false negative.
 > textarea (frontend UX) — the two are independent of the rest of the
 > panel/view CRUD.
 
+### Authorization
+
+Access to the dashboard *page* is gated separately from access to individual
+*data*. Two custom permissions, declared in `plugin.json` and each available
+at both project and global **permission** scope (the same key checked
+against two different permission maps, mirroring `paca-plugin-time-logging`).
+This is the only "scope" the permission system itself has —
+`requirePermissions`/`customPermissions` entries are always either
+`"project"` (checked against the caller's per-project permission map) or
+`"global"` (checked against their global permission map). There is no third,
+"admin" permission scope, even though the paragraphs below also need to talk
+about `dashboard_views.scope`, an unrelated data-model column on this
+plugin's own table that happens to have a value literally named `"admin"` —
+every occurrence of "admin" below refers to that column's value or to the
+`/admin/*`-routed pages, never to a permission scope:
+
+- **`dashboard.view`** — see the project's (or admin's) Dashboard page and
+  its panels.
+- **`dashboard.manage`** — create, edit, delete, and rearrange panels, and
+  run query previews while authoring them. Implies `dashboard.view` for
+  loading purposes — a role with only `dashboard.manage` isn't blocked from
+  the page itself.
+
+Every project-*permission*-scope backend route requires `dashboard.view` at
+minimum, declared directly in the manifest's `requirePermissions` —
+including the routes shared with dashboards whose `dashboard_views.scope`
+is `"integration"` (embedded in Backlog/Sprint/Timeline, see "Integration
+view architecture" above), which the manifest can't distinguish from the
+`dashboard_views.scope == "project"` singleton by path alone. There is no
+"always open" carve-out for the embedded surface: `dashboard.view` gates
+the whole feature uniformly, project page and embedded views alike.
+`views.go`'s `loadViewWithPanels` and `getOrCreate*View` handlers re-check
+this in-handler too, as defense-in-depth against the manifest ever being
+misedited — not because the manifest leaves any gap today.
+
+On top of that `dashboard.view` floor, mutating a panel requires a second,
+narrower check in `panels.go`'s `canManagePanel`, which resolves the
+request's `dashboard_views.scope` value and picks the (project-permission-
+scope) permission that matches:
+
+- row's `dashboard_views.scope` is `"project"`: requires `dashboard.manage`.
+- row's `dashboard_views.scope` is `"integration"`: requires `views.write`
+  instead of `dashboard.manage` — an integration dashboard is embedded in,
+  and shares the write bar of, the view that hosts it. `views.write` is
+  exactly the permission that already lets an ordinary project Editor
+  create that hosting view in the first place; requiring anything higher
+  here would be a dead end (create the view, never populate it).
+- row's `dashboard_views.scope` is `"admin"`: never reaches this function —
+  those routes are already fully gated at the manifest level by
+  `dashboard.manage` at **permission** scope `"global"`.
+
+This two-layer shape (`dashboard.view` as a uniform manifest-level floor,
+a data-scope-specific manage permission checked in-handler on top) is
+deliberate: `requirePermissions` ANDs every permission it lists, so putting
+`dashboard.manage` directly in the manifest on a route shared with the
+`"integration"` data scope would reject a `views.write`-holding Editor
+before the in-handler check ever ran.
+
+`POST /dashboard/query/preview` (project permission scope) has no
+`dashboard_views` row to resolve a data scope from — it validates a
+not-yet-saved query directly against the caller's project, and is used by
+both the project-dashboard editor and the integration-view editor with no
+way to tell which from the request alone. Past the manifest's
+`dashboard.view` floor, it requires
+`dashboard.manage` in-handler (matching that permission's own description,
+which promises coverage of "running query previews while authoring"
+panels). The one remaining, accepted gap: a hand-crafted custom role
+granted only `views.write` (able to populate an integration dashboard)
+can't preview a query for it through this route — closing that fully would
+need the frontend to pass a `viewId` so the backend can resolve scope, a
+small API contract change tracked as a follow-up rather than done here.
+
 ### MCP (`mcp/`)
 
 `mcp/src/index.ts` exposes the panel/view API to AI clients (Claude, GitHub
@@ -309,20 +386,19 @@ bun run build
 ### `project.page` — `ProjectDashboardPage`
 
 Routed at `/projects/:projectId/plugins/com.paca.dashboard/dashboard` via
-the `project` nav item declared in `plugin.json`. Fetches (get-or-creates)
-the project's singleton dashboard view and renders the shared panel-grid
-UI with edit permissions gated by the route's own `projects.write`
-middleware (panel mutation routes require `projects.write`; read routes
-require only `projects.read`).
+the `project` nav item declared in `plugin.json`, requiring the
+`dashboard.view` custom permission (see "Authorization" above). Fetches
+(get-or-creates) the project's singleton dashboard view and renders the
+shared panel-grid UI, with panel mutations requiring `dashboard.manage`.
 
 ### `admin.page` — `AdminDashboardPage`
 
 Routed at `/admin/plugins/com.paca.dashboard/dashboard` via the `admin` nav
-item declared in `plugin.json`; gated by the built-in `users.write` global
-permission (same gate as the host's other admin pages). Fetches
-(get-or-creates) the instance-wide singleton dashboard view; its panel
-queries are cross-project and do not require the `{{project_id}}`
-placeholder.
+item declared in `plugin.json`, requiring the global-scope `dashboard.view`
+custom permission (panel mutations require `dashboard.manage`) — see
+"Authorization" above. Fetches (get-or-creates) the instance-wide singleton
+dashboard view; its panel queries are cross-project and do not require the
+`{{project_id}}` placeholder.
 
 ### `view` — `DashboardIntegrationView`
 

backend/panels.go

@@ -131,6 +131,57 @@ func (p *dashboardPlugin) previewAdminQuery(req *plugin.Request, res *plugin.Res
 	p.previewQuery(req, res, false)
 }
 
+// canManagePanel reports whether the caller may create/edit/delete/reorder
+// panels on a view whose dashboard_views.scope column holds viewScope
+// ("project" | "integration" | "admin" — a data-model value describing
+// which kind of dashboard the row is, NOT a permission scope; the
+// permission system itself only ever has two scopes, "project" and
+// "global", set on the requirePermissions/customPermissions entries in
+// plugin.json). A single permission decides the answer here — not two —
+// which is exactly why the manifest's own requirePermissions floor on
+// these routes is deliberately just projects.read rather than
+// projects.write: requirePermissions ANDs every listed permission, so if
+// the manifest floor were projects.write, a role granted only
+// dashboard.manage would 403 at the middleware layer before this in-handler
+// check ever ran, silently defeating the point of a dedicated permission.
+//
+//   - viewScope == "project": requires dashboard.manage (a project-scope
+//     permission, i.e. checked against the caller's per-project permission
+//     map).
+//   - viewScope == "integration": requires views.write instead of
+//     dashboard.manage — an integration dashboard is embedded in, and
+//     shares the write bar of, the view that hosts it. views.write is
+//     exactly the permission that already lets an ordinary Editor create
+//     that hosting view in the first place; requiring anything higher here
+//     would be a dead end (create the view, never populate it). Also a
+//     project-scope permission.
+//   - viewScope == "admin": never reaches this function — loadViewWithPanels
+//     only lets projectID == "" through the admin routes, which are already
+//     fully gated at the manifest level by dashboard.manage checked at
+//     permission scope "global" (not "admin" — there is no such permission
+//     scope).
+func (p *dashboardPlugin) canManagePanel(viewScope string) bool {
+	if viewScope == "integration" {
+		return p.perm.Check("views.write")
+	}
+	return p.perm.Check("dashboard.manage")
+}
+
+// requireManagePanel enforces canManagePanel for view, writing the shared
+// 403 response and returning false when the caller may not manage it — the
+// same check every panel-mutation handler below needs before touching
+// view's panels. Admin-scope views skip the check entirely (loadViewWithPanels
+// only lets projectID == "" through the admin routes, which are already
+// fully gated at the manifest level), matching canManagePanel's own doc
+// comment on why viewScope == "admin" never reaches canManagePanel itself.
+func (p *dashboardPlugin) requireManagePanel(view *dashboardView, res *plugin.Response) bool {
+	if view.Scope != "admin" && !p.canManagePanel(view.Scope) {
+		res.Error(403, "you don't have permission to manage this dashboard")
+		return false
+	}
+	return true
+}
+
 // ── Shared implementations ───────────────────────────────────────────────────
 
 // createPanelForView handles POST .../:viewId/panels for any scope.
@@ -144,6 +195,9 @@ func (p *dashboardPlugin) createPanelForView(req *plugin.Request, res *plugin.Re
 	if !loaded {
 		return
 	}
+	if !p.requireManagePanel(view, res) {
+		return
+	}
 
 	b, err := plugin.JSONBody[panelBody](req)
 	if err != nil {
@@ -196,6 +250,9 @@ func (p *dashboardPlugin) updatePanelForView(req *plugin.Request, res *plugin.Re
 	if !loaded {
 		return
 	}
+	if !p.requireManagePanel(view, res) {
+		return
+	}
 	if !p.panelBelongsToView(panelID, viewID, res) {
 		return
 	}
@@ -261,7 +318,11 @@ func (p *dashboardPlugin) deletePanelForView(req *plugin.Request, res *plugin.Re
 		return
 	}
 	panelID := req.PathParam("panelId")
-	if _, ok2 := p.loadViewWithPanels(viewID, projectID, res); !ok2 {
+	view, loaded := p.loadViewWithPanels(viewID, projectID, res)
+	if !loaded {
+		return
+	}
+	if !p.requireManagePanel(view, res) {
 		return
 	}
 	if !p.panelBelongsToView(panelID, viewID, res) {
@@ -290,7 +351,11 @@ func (p *dashboardPlugin) updatePanelLayoutForView(req *plugin.Request, res *plu
 	if !viewOK {
 		return
 	}
-	if _, ok2 := p.loadViewWithPanels(viewID, projectID, res); !ok2 {
+	view, loaded := p.loadViewWithPanels(viewID, projectID, res)
+	if !loaded {
+		return
+	}
+	if !p.requireManagePanel(view, res) {
 		return
 	}
 
@@ -390,6 +455,17 @@ func (p *dashboardPlugin) runPanelQueryForView(req *plugin.Request, res *plugin.
 // equivalent) — validates and runs a not-yet-saved query so the panel
 // editor can show a live preview before the user hits Save.
 func (p *dashboardPlugin) previewQuery(req *plugin.Request, res *plugin.Response, requireProjectScope bool) {
+	// The admin path (requireProjectScope == false) is already fully gated
+	// at the manifest level (dashboard.manage, global scope) — no view to
+	// resolve a scope from here, so this only applies the project-scope
+	// check. dashboard.manage's own description promises it covers "running
+	// query previews while authoring" panels, so this must hold even though
+	// the manifest's own floor is the non-restrictive projects.read (same
+	// reasoning as canManagePanel).
+	if requireProjectScope && !p.canManagePanel("project") {
+		res.Error(403, "you don't have permission to manage this dashboard")
+		return
+	}
 	type previewQueryBody struct {
 		Query string `json:"query"`
 	}

backend/panels_cache_test.go

@@ -132,22 +132,36 @@ func TestUpdatePanel_InvalidatesCachedData(t *testing.T) {
 
 	// Narrowing the panel's query must invalidate its cached result —
 	// otherwise this would keep serving the old 2-row result for up to
-	// panelDataCacheTTL after the edit. The {{project_id}} placeholder is
-	// substituted for every occurrence of $1 (see query_guard.go), so
-	// repeating it as a second condition against `id` — which never equals
-	// the project_id string — deterministically narrows the seeded 2-row
-	// result to 0 without needing a second bindable parameter.
-	tc.Call("PATCH", "/dashboard/views/:viewId/panels/:panelId",
+	// panelDataCacheTTL after the edit. query_guard.go now requires the
+	// {{project_id}} placeholder to be used as a direct "project_id =
+	// {{project_id}}" equality filter and rejects comparing it against any
+	// other column (see validateProjectScopePlaceholder /
+	// placeholderComparisonRe) — so the previous version of this test,
+	// which compared it against tasks.id to force a deterministic 0-row
+	// result, is no longer a valid panel query. That's exactly the class of
+	// decoy filter the guard now closes (a placeholder-equality against an
+	// unrelated column doesn't actually scope the returned rows), so this
+	// test needs a different way to get a deterministic, different-from-
+	// before result. Querying dashboard_views instead of tasks does that
+	// while still using a single valid `project_id = {{project_id}}`
+	// filter: this project has exactly one dashboard_views row (the view
+	// fetched at the top of this test), versus tasks' two, and
+	// plugintest.InMemoryDB's minimal parser (col = $N chains only) has no
+	// trouble with it.
+	patchRes := tc.Call("PATCH", "/dashboard/views/:viewId/panels/:panelId",
 		withPathParams(callerReq(), map[string]string{"viewId": view.ID, "panelId": panel.ID}).
 			WithJSONBody(map[string]any{
 				"type":  "table",
 				"title": "My tasks",
-				"query": "SELECT id, title FROM tasks WHERE project_id = {{project_id}} AND id = {{project_id}}",
+				"query": "SELECT id, name FROM dashboard_views WHERE project_id = {{project_id}}",
 			}))
+	if patchRes.StatusCode != 200 {
+		t.Fatalf("expected 200 updating the panel's query, got %d: %s", patchRes.StatusCode, patchRes.BodyString())
+	}
 
 	second := panelDataRows(t, tc.Call("POST", "/dashboard/views/:viewId/panels/:panelId/data", dataReq))
-	if len(second) != 0 {
-		t.Fatalf("expected the updated query's 0-row result after cache invalidation, got %d: %+v", len(second), second)
+	if len(second) != 1 {
+		t.Fatalf("expected the updated query's 1-row result (this project's own dashboard_views row) after cache invalidation, got %d: %+v", len(second), second)
 	}
 }
 

backend/plugin.go

@@ -36,13 +36,15 @@ type dashboardPlugin struct {
 	db    *plugin.DB
 	cache *plugin.Cache
 	log   *plugin.Logger
+	perm  *plugin.Permissions
 }
 
 // Init registers all routes on the provided context.
 func (p *dashboardPlugin) Init(ctx *plugin.Context) error {
 	p.db = ctx.DB()
 	p.cache = ctx.Cache()
 	p.log = ctx.Log()
+	p.perm = ctx.Permissions()
 
 	// ── Project-scope dashboard (get-or-create singleton) ──────────────────
 	ctx.Route("GET", "/dashboard/view", p.getOrCreateProjectView)