From 89499590c0355cef7cda4be42ea61044d75bc052 Mon Sep 17 00:00:00 2001 From: pikann22 Date: Mon, 14 Sep 2026 08:35:43 +0000 Subject: [PATCH 1/2] feat: split dashboard.view/dashboard.manage permissions; harden query guard Every dashboard route was gated on broad, unrelated permissions (projects.read/projects.write/users.write) instead of anything dashboard-specific. Introduced dashboard.view and dashboard.manage custom permissions, each available at both project and global scope, and gated every route and the sidebar nav entries with them. dashboard.manage implies dashboard.view for loading purposes. Also hardens validateProjectScopePlaceholder in query_guard.go: the {{project_id}} filter previously just had to appear somewhere in a WHERE/ON/HAVING clause, which a duplicated/self-compared placeholder, a non-equality operator, or a sibling bare OR could all short-circuit into matching across every project. It must now appear exactly once, as a direct equality filter, with no bare OR anywhere in the query. Co-Authored-By: Claude Sonnet 5 --- README.md | 108 ++++++++++++--- backend/panels.go | 80 ++++++++++- backend/panels_cache_test.go | 32 +++-- backend/plugin.go | 2 + backend/plugin_test.go | 240 +++++++++++++++++++++++++++++++++ backend/query_guard.go | 248 ++++++++++++++++++++++++++++++++--- backend/views.go | 45 +++++++ plugin.json | 64 ++++++--- 8 files changed, 759 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index a06b33e..5018dc8 100644 --- a/README.md +++ b/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 ` = {{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` diff --git a/backend/panels.go b/backend/panels.go index fb8aa2d..b4f609e 100644 --- a/backend/panels.go +++ b/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"` } diff --git a/backend/panels_cache_test.go b/backend/panels_cache_test.go index e4806dc..6424cb7 100644 --- a/backend/panels_cache_test.go +++ b/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) } } diff --git a/backend/plugin.go b/backend/plugin.go index 53fb278..279c6b0 100644 --- a/backend/plugin.go +++ b/backend/plugin.go @@ -36,6 +36,7 @@ type dashboardPlugin struct { db *plugin.DB cache *plugin.Cache log *plugin.Logger + perm *plugin.Permissions } // Init registers all routes on the provided context. @@ -43,6 +44,7 @@ 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) diff --git a/backend/plugin_test.go b/backend/plugin_test.go index 3cfa84e..7109122 100644 --- a/backend/plugin_test.go +++ b/backend/plugin_test.go @@ -4,6 +4,7 @@ package main import ( "encoding/json" + "strings" "testing" plugin "github.com/Paca-AI/plugin-sdk-go" @@ -17,6 +18,11 @@ const testProjectID = "project-1" func setupPlugin(t *testing.T) *plugintest.Context { t.Helper() tc := plugintest.NewContext(t) + // Default test caller has full dashboard access; tests for the + // permission gate itself (TestLoadView_RequiresDashboardView etc.) + // explicitly revoke these to exercise the denied path. + tc.Permissions.Grant("dashboard.view") + tc.Permissions.Grant("dashboard.manage") tc.DB.SeedRows("dashboard_views", []string{"id", "project_id", "scope", "host_view_id", "name", "created_by", "created_at", "updated_at"}, @@ -255,6 +261,66 @@ func TestGetView_CrossProjectRejected(t *testing.T) { } } +// ── Dashboard permission gate (dashboard.view / dashboard.manage) ─────────── + +func TestGetOrCreateProjectView_RequiresDashboardView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("GET", "/dashboard/view", callerReq()) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller with neither dashboard.view nor dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +func TestGetOrCreateProjectView_ManageAloneIsSufficientToView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + // dashboard.manage stays granted (setupPlugin's default) — manage alone + // must still be enough to load the view. + + res := tc.Call("GET", "/dashboard/view", callerReq()) + if res.StatusCode != 200 { + t.Fatalf("expected 200 for a caller with only dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +// TestGetOrCreateIntegrationView_RequiresDashboardView pins the current +// design: dashboard.view gates every dashboard surface uniformly, including +// integration-scope dashboards embedded in Backlog/Sprint/Timeline — not +// just the dedicated project-scope singleton page. Revoking dashboard.view +// from a role now hides the whole feature, embedded views included; there +// is no "always open" carve-out for the embedded surface. +func TestGetOrCreateIntegrationView_RequiresDashboardView(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.view") + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("GET", "/dashboard/view/:hostViewId", + withPathParams(callerReq(), map[string]string{"hostViewId": testHostViewID})) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for an integration-scope view without dashboard.view, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +func TestCreatePanel_RequiresDashboardManage(t *testing.T) { + tc := setupPlugin(t) + view := decodeData[dashboardView](t, tc.Call("GET", "/dashboard/view", callerReq())) + tc.Permissions.Revoke("dashboard.manage") // dashboard.view stays granted + + res := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{ + "type": "text", + "title": "Notes", + "content": "should be rejected before it's ever validated", + })) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller with dashboard.view but not dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + // ── Panel CRUD ──────────────────────────────────────────────────────────────── func TestCreateTextPanel(t *testing.T) { @@ -501,6 +567,57 @@ func TestRunPanelQuery_ScopesToOwnProject(t *testing.T) { } } +// TestPreviewQuery_RequiresDashboardManage pins the fix for the +// query/preview route relying solely on the manifest's projects.read floor +// with no in-handler check at all — dashboard.manage's own description +// promises it covers "running query previews while authoring" panels, so a +// caller without it must be rejected here too, not just on save. +func TestPreviewQuery_RequiresDashboardManage(t *testing.T) { + tc := setupPlugin(t) + tc.Permissions.Revoke("dashboard.manage") + + res := tc.Call("POST", "/dashboard/query/preview", + callerReq().WithJSONBody(map[string]string{ + "query": "SELECT id FROM tasks WHERE project_id = {{project_id}}", + })) + if res.StatusCode != 403 { + t.Fatalf("expected 403 for a caller without dashboard.manage, got %d: %s", res.StatusCode, res.BodyString()) + } +} + +// TestCreatePanel_IntegrationScope_RequiresViewsWrite pins the fix for +// integration-scope panel mutations previously having no permission check +// at all in-handler (only the manifest's projects.write, which an ordinary +// Editor — who can create the hosting integration view via views.write — +// doesn't hold, making the view creatable but never populatable). Panel +// mutations on an integration view now require views.write specifically, +// not dashboard.manage — matching the permission that already gates +// creating the view itself. +func TestCreatePanel_IntegrationScope_RequiresViewsWrite(t *testing.T) { + tc := setupPlugin(t) + view := decodeData[dashboardView](t, tc.Call("GET", "/dashboard/view/:hostViewId", + withPathParams(callerReq(), map[string]string{"hostViewId": testHostViewID}))) + + // dashboard.manage alone must NOT be sufficient for an integration view. + tc.Permissions.Revoke("views.write") + denied := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{"type": "text", "title": "Notes", "content": "x"})) + if denied.StatusCode != 403 { + t.Fatalf("expected 403 for an integration view without views.write, got %d: %s", denied.StatusCode, denied.BodyString()) + } + + // views.write alone (without dashboard.manage) must be sufficient. + tc.Permissions.Revoke("dashboard.manage") + tc.Permissions.Grant("views.write") + allowed := tc.Call("POST", "/dashboard/views/:viewId/panels", + withPathParams(callerReq(), map[string]string{"viewId": view.ID}). + WithJSONBody(map[string]any{"type": "text", "title": "Notes", "content": "x"})) + if allowed.StatusCode != 201 { + t.Fatalf("expected 201 for an integration view with views.write but not dashboard.manage, got %d: %s", allowed.StatusCode, allowed.BodyString()) + } +} + func TestPreviewQuery_RejectsForbiddenKeyword(t *testing.T) { tc := setupPlugin(t) @@ -588,6 +705,129 @@ func TestValidateQuery_RejectsDollarOneDirectUse(t *testing.T) { } } +// TestValidateQuery_RejectsDuplicatedPlaceholderTautology pins the fix for a +// real cross-project data leak: the old guard only checked that +// {{project_id}} appeared *somewhere* in the query, so a self-comparison +// like this one passed validation and became an always-true "$1 = $1" +// filter after substitution — returning every row in the table, across +// every project, regardless of the caller's own project_id. +func TestValidateQuery_RejectsDuplicatedPlaceholderTautology(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE '{{project_id}}' = '{{project_id}}'", true); err == nil { + t.Fatal("expected error: duplicated placeholder forms an always-true tautology") + } +} + +// TestValidateQuery_RejectsOrShortCircuit pins the fix for the second +// concrete bypass the old "contains the token somewhere" check allowed: a +// correctly-used placeholder rendered moot by a sibling top-level OR, which +// would return rows regardless of project since the OR's other branch is +// always true. +func TestValidateQuery_RejectsOrShortCircuit(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE 1=1 OR project_id = {{project_id}}", true); err == nil { + t.Fatal("expected error: a top-level OR can bypass the project filter") + } +} + +// TestValidateQuery_RejectsNonEqualityPlaceholderUse guards against a +// variant of the same class of bug: a comparison operator other than "=" +// (e.g. "!=") inverts the filter into "every other project" instead of +// "just mine". +func TestValidateQuery_RejectsNonEqualityPlaceholderUse(t *testing.T) { + if _, err := validateQuery("SELECT id FROM tasks WHERE project_id != {{project_id}}", true); err == nil { + t.Fatal("expected error: != inverts the project filter instead of scoping to it") + } +} + +// TestValidateQuery_AllowsOrInsideStringLiteral guards against a +// too-aggressive fix: "OR" appearing as ordinary text inside a quoted +// string literal (not the SQL keyword) must not trip the OR ban. +func TestValidateQuery_AllowsOrInsideStringLiteral(t *testing.T) { + safe, err := validateQuery( + "SELECT id FROM tasks WHERE project_id = {{project_id}} AND title = 'Manager or Director'", true) + if err != nil { + t.Fatalf("unexpected error for OR inside a string literal: %v", err) + } + if !strings.Contains(safe, "Manager or Director") { + t.Fatalf("expected the string literal to survive validation unchanged, got %q", safe) + } +} + +// TestValidateQuery_RejectsUnion pins the fix for a concrete cross-project +// leak: none of the placeholder/OR checks understand a second SELECT +// contributing rows via UNION, so a properly-scoped first branch used to be +// enough to pass validation while an unfiltered second branch returned +// every project's rows. +func TestValidateQuery_RejectsUnion(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks WHERE project_id = {{project_id}} UNION SELECT id, title FROM tasks", true); err == nil { + t.Fatal("expected error: UNION can append unfiltered rows from a second SELECT") + } +} + +// TestValidateQuery_RejectsPlaceholderInsideSubquery pins the fix for a +// decoy filter: a placeholder-equality that satisfies a naive text search +// while sitting inside an unrelated nested subquery doesn't actually +// constrain which rows the outer query returns. +func TestValidateQuery_RejectsPlaceholderInsideSubquery(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks WHERE (SELECT 1 FROM projects WHERE id = {{project_id}}) IS NOT NULL", true); err == nil { + t.Fatal("expected error: placeholder used inside a nested subquery doesn't filter the outer query's rows") + } +} + +// TestValidateQuery_RejectsPlaceholderAgainstWrongColumn pins the fix for a +// decoy cross join: comparing the placeholder against an unrelated column +// (here projects.id, a different table's primary key) leaves the actually +// -returned table's rows completely unfiltered. +func TestValidateQuery_RejectsPlaceholderAgainstWrongColumn(t *testing.T) { + if _, err := validateQuery( + "SELECT id, title FROM tasks, projects WHERE projects.id = {{project_id}}", true); err == nil { + t.Fatal("expected error: placeholder must filter a project_id column, not an unrelated column") + } +} + +// TestValidateQuery_RejectsOrHiddenByQuotedIdentifier pins the fix for a +// string-tracking desync: an apostrophe inside a double-quoted identifier +// (valid SQL, e.g. an alias) used to desync the old single-quote-only +// literal tracker, making everything after it invisible to the OR check. +func TestValidateQuery_RejectsOrHiddenByQuotedIdentifier(t *testing.T) { + if _, err := validateQuery( + `SELECT id, 1 AS "it's" FROM tasks WHERE project_id = {{project_id}} OR 1=1`, true); err == nil { + t.Fatal("expected error: a bare OR must still be caught after a double-quoted identifier containing an apostrophe") + } +} + +// TestValidateQuery_AllowsQualifiedProjectIdInJoin guards against a +// too-aggressive fix: an ordinary explicit JOIN, with the placeholder +// filtering an alias-qualified project_id column, is the documented +// supported shape and must keep working. +func TestValidateQuery_AllowsQualifiedProjectIdInJoin(t *testing.T) { + safe, err := validateQuery( + "SELECT t.id, u.username FROM tasks t JOIN users u ON u.id = t.assignee_id WHERE t.project_id = {{project_id}}", true) + if err != nil { + t.Fatalf("unexpected error for an alias-qualified project_id filter in a joined query: %v", err) + } + if !containsLimit(safe) { + t.Fatalf("expected LIMIT to be appended, got %q", safe) + } +} + +// TestValidateQuery_AllowsCteWithTopLevelFilter guards against a +// too-aggressive fix: a WITH/CTE query is a documented supported shape as +// long as the project_id filter is in the outer query's own WHERE, which is +// what every real usage pattern in this file does. +func TestValidateQuery_AllowsCteWithTopLevelFilter(t *testing.T) { + safe, err := validateQuery( + "WITH recent AS (SELECT id, title FROM tasks ORDER BY created_at DESC) "+ + "SELECT * FROM recent WHERE project_id = {{project_id}}", true) + if err != nil { + t.Fatalf("unexpected error for a CTE with the placeholder filter at the outer top level: %v", err) + } + if !containsLimit(safe) { + t.Fatalf("expected LIMIT to be appended, got %q", safe) + } +} + func containsLimit(sql string) bool { return limitRe.MatchString(sql) } diff --git a/backend/query_guard.go b/backend/query_guard.go index a8b11a6..2883267 100644 --- a/backend/query_guard.go +++ b/backend/query_guard.go @@ -16,14 +16,19 @@ // functions, dblink, lo_*, or COPY — closes the standard sandbox-escape // tricks for a restricted SQL surface. // 3. Project-scoped queries (project + integration dashboard scopes) must -// contain the literal placeholder token `{{project_id}}` somewhere in -// a WHERE/ON/HAVING clause; the guard substitutes it with `$1` and the -// caller's real project_id is bound as that parameter. A query with no -// `{{project_id}}` placeholder is rejected outright for these scopes — -// this is what actually prevents cross-project data leakage, since it -// forces every project-scoped query to filter by the caller's own -// project. Admin-scope queries are intentionally cross-project and skip -// this requirement. +// use the literal placeholder token `{{project_id}}` exactly once, as a +// direct ` = {{project_id}}` (or reversed) equality filter, with +// no bare OR anywhere in the query; the guard then substitutes it with +// `$1` and the caller's real project_id is bound as that parameter (see +// validateProjectScopePlaceholder). A query with no `{{project_id}}` +// placeholder, or one that only *contains* the token without using it as +// a genuine per-row filter (duplicated into a self-comparison, compared +// with anything but `=`, or otherwise made bypassable via OR), is +// rejected outright for these scopes — this is what actually prevents +// cross-project data leakage, since it forces every project-scoped query +// to filter by the caller's own project in a way that can't be +// short-circuited. Admin-scope queries are intentionally cross-project +// and skip this requirement. // 4. An explicit LIMIT is appended (LIMIT 500) when the query doesn't // already declare one, bounding worst-case result size / query cost. // @@ -52,8 +57,13 @@ import ( ) // forbiddenKeywords are rejected anywhere in the query (case-insensitive, -// word-bounded) — mutating statements, introspection, and known -// sandbox-escape surfaces. +// word-bounded) — mutating statements, introspection, known +// sandbox-escape surfaces, and compound-SELECT operators. The latter close +// a real bypass: none of the placeholder/OR checks below understand +// multiple SELECTs contributing rows to one result, so +// "... WHERE project_id={{project_id}} UNION SELECT ... FROM tasks" (no +// filter on the second branch) satisfied every other check and returned +// every project's rows. var forbiddenKeywords = []string{ "insert", "update", "delete", "drop", "alter", "create", "grant", "revoke", "truncate", "copy", "call", "execute", "explain", "vacuum", @@ -61,6 +71,7 @@ var forbiddenKeywords = []string{ "pg_sleep", "pg_read_file", "pg_read_binary_file", "pg_ls_dir", "dblink", "lo_import", "lo_export", "information_schema", "pg_catalog", "pg_shadow", "pg_authid", "current_setting", "set_config", + "union", "intersect", "except", } var identWordRe = regexp.MustCompile(`[A-Za-z_][A-Za-z0-9_]*`) @@ -81,6 +92,216 @@ func invalidQuery(format string, args ...any) error { return &queryValidationError{msg: fmt.Sprintf(format, args...)} } +// placeholderComparisonRe matches "project_id = {{project_id}}" or +// "{{project_id}} = project_id" (optionally table/alias-qualified, e.g. +// "t.project_id"), run against the *redacted* query text (see +// redactForStructuralChecks) rather than the raw query. Two things narrow +// this beyond a bare "some column somewhere": +// +// - The column name must literally be project_id, not any identifier. +// Every real use in this file's own docs/tests filters on project_id; +// requiring the name closes a decoy like +// "FROM tasks, projects WHERE projects.id = {{project_id}}" — a +// cross join whose only placeholder use filters an unrelated table's +// primary key, leaving the actually-returned tasks rows unfiltered. +// - Running against the redacted text means a match can't be hiding +// inside a string literal or a nested subquery — see +// redactForStructuralChecks's doc comment for why the latter matters. +var placeholderComparisonRe = regexp.MustCompile( + `(?i)(?:[A-Za-z_][A-Za-z0-9_]*\.)?project_id\s*=\s*` + regexp.QuoteMeta(placeholderToken) + + `|` + regexp.QuoteMeta(placeholderToken) + `\s*=\s*(?:[A-Za-z_][A-Za-z0-9_]*\.)?project_id`, +) + +// validateProjectScopePlaceholder enforces that {{project_id}} appears +// exactly once and is used as a direct, unconditional equality filter +// against a real column — closing two concrete bypasses that a bare +// "contains the placeholder somewhere" check allows through: +// +// - A duplicated/self-compared placeholder, e.g. +// "WHERE '{{project_id}}' = '{{project_id}}'", which becomes an +// always-true tautology once both sides are substituted with the same +// bound value — the old check only verified the token appeared +// *somewhere*, so this passed. +// - The placeholder appearing correctly, but alongside a bare OR that can +// make the whole filter match regardless of project, e.g. +// "WHERE 1=1 OR project_id = {{project_id}}". Reasoning about which +// OR/parenthesis nestings are "safe" would require a real SQL parser +// (a parenthesized OR can still be just as unsafe as a bare one, e.g. +// "WHERE (other_col = 5 OR project_id = {{project_id}})" — the parens +// don't stop it from being the entire filter), so OR is rejected +// outright for project/integration-scoped queries rather than +// attempting to classify individual uses as safe. Use IN (...) for a +// multi-value filter instead. +func validateProjectScopePlaceholder(query string) error { + count := strings.Count(query, placeholderToken) + if count == 0 { + return invalidQuery( + "query must scope itself to the current project using the %s placeholder somewhere in a WHERE/ON/HAVING clause (e.g. \"WHERE project_id = %s\")", + placeholderToken, placeholderToken, + ) + } + if count > 1 { + return invalidQuery("the %s placeholder must appear exactly once", placeholderToken) + } + + redacted := redactForStructuralChecks(query) + + if hasBareOr(redacted) { + return invalidQuery("OR is not allowed in panel queries, since it could bypass the project filter — use IN (...) for a multi-value filter instead") + } + if !placeholderComparisonRe.MatchString(redacted) { + return invalidQuery( + "the %s placeholder must be used as a direct equality filter against a project_id column, at the query's own top level (not inside a nested subquery), e.g. \"WHERE project_id = %s\" — it cannot be compared with anything other than \"=\", against a different column, duplicated, or embedded inside a string literal or larger expression", + placeholderToken, placeholderToken, + ) + } + return nil +} + +// redactForStructuralChecks returns a same-length copy of query with the +// content of every single-quoted string literal, double-quoted identifier, +// and nested SELECT/WITH subquery (anything inside parentheses that open +// with SELECT or WITH, at any depth) blanked out to spaces. hasBareOr and +// placeholderComparisonRe both run against this redacted form instead of +// the raw query, for two independent reasons: +// +// - String-literal content can contain a stray quote, paren, or +// keyword-looking text that isn't SQL syntax at all. +// - A nested subquery can contain a placeholder-equality that satisfies +// placeholderComparisonRe by pure text matching without actually +// constraining which rows the *outer* query returns — e.g. +// "SELECT id,title FROM tasks WHERE (SELECT 1 FROM projects WHERE +// id={{project_id}}) IS NOT NULL" filters on an unrelated existence +// check while every row of tasks (every project's) comes back +// unfiltered. Blanking nested-subquery spans means a placeholder +// hiding in one no longer counts as a valid top-level filter, so this +// query now correctly falls through to "no placeholder used as a +// direct equality filter" and is rejected. Legitimate queries are +// expected to place the filter in their own top-level WHERE (every +// example in this file's docs/tests already does), so this doesn't +// restrict the documented usage pattern. +func redactForStructuralChecks(query string) string { + runes := []rune(query) + out := make([]rune, len(runes)) + copy(out, runes) + + inSingle, inDouble := false, false + var subqueryParen []bool // one entry per currently-open paren; true if it opens a nested SELECT/WITH + + insideSubquery := func() bool { + for _, sq := range subqueryParen { + if sq { + return true + } + } + return false + } + + for i := 0; i < len(runes); i++ { + c := runes[i] + + if inSingle { + if c == '\'' { + if i+1 < len(runes) && runes[i+1] == '\'' { + out[i], out[i+1] = ' ', ' ' + i++ + continue + } + inSingle = false + continue + } + out[i] = ' ' + continue + } + if inDouble { + if c == '"' { + if i+1 < len(runes) && runes[i+1] == '"' { + out[i], out[i+1] = ' ', ' ' + i++ + continue + } + inDouble = false + continue + } + out[i] = ' ' + continue + } + + switch c { + case '\'': + inSingle = true + continue + case '"': + inDouble = true + continue + case '(': + subqueryParen = append(subqueryParen, startsWithSelectOrWith(runes[i+1:])) + continue + case ')': + if len(subqueryParen) > 0 { + subqueryParen = subqueryParen[:len(subqueryParen)-1] + } + continue + } + + if insideSubquery() { + out[i] = ' ' + } + } + return string(out) +} + +// startsWithSelectOrWith reports whether after, everything immediately +// following an opening paren, begins (modulo leading whitespace) with the +// keyword SELECT or WITH — i.e. whether that paren opens a nested +// subquery/CTE rather than a plain grouping expression like +// "(project_id = {{project_id}})" or "(a = 1 AND b = 2)". +func startsWithSelectOrWith(after []rune) bool { + i := 0 + for i < len(after) && (after[i] == ' ' || after[i] == '\t' || after[i] == '\n' || after[i] == '\r') { + i++ + } + return isWordBoundaryKeyword(after, i, "select") || isWordBoundaryKeyword(after, i, "with") +} + +// hasBareOr reports whether query contains the keyword OR outside of a +// string literal (single- or double-quoted — see redactForStructuralChecks, +// which callers are expected to have already applied) and not as part of a +// longer identifier (e.g. "corporation", "order"). +func hasBareOr(query string) bool { + runes := []rune(query) + for i := 0; i < len(runes); i++ { + if isWordBoundaryKeyword(runes, i, "or") { + return true + } + } + return false +} + +// isWordBoundaryKeyword reports whether the case-insensitive keyword kw +// occurs at position i in runes, bounded by non-identifier characters (or +// the string's edges) on both sides. +func isWordBoundaryKeyword(runes []rune, i int, kw string) bool { + n := len(kw) + if i+n > len(runes) { + return false + } + if !strings.EqualFold(string(runes[i:i+n]), kw) { + return false + } + if i > 0 && isIdentRune(runes[i-1]) { + return false + } + if i+n < len(runes) && isIdentRune(runes[i+n]) { + return false + } + return true +} + +func isIdentRune(r rune) bool { + return r == '_' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') +} + // validateQuery checks a raw panel query string against the safety model // described above. requireProjectScope is true for 'project' and // 'integration' scope panels (false for 'admin' scope, which is @@ -121,11 +342,8 @@ func validateQuery(raw string, requireProjectScope bool) (string, error) { } if requireProjectScope { - if !strings.Contains(trimmed, placeholderToken) { - return "", invalidQuery( - "query must scope itself to the current project using the %s placeholder somewhere in a WHERE/ON/HAVING clause (e.g. \"WHERE project_id = %s\")", - placeholderToken, placeholderToken, - ) + if err := validateProjectScopePlaceholder(trimmed); err != nil { + return "", err } // $1 is reserved for the injected project_id; reject any other // use of $1 so we don't silently override a user's own parameter. diff --git a/backend/views.go b/backend/views.go index 9c0ec4d..7acae32 100644 --- a/backend/views.go +++ b/backend/views.go @@ -23,6 +23,15 @@ import ( // /projects/:projectId/dashboard/view via the manifest). Returns the // project's single dashboard, creating an empty one on first visit. func (p *dashboardPlugin) getOrCreateProjectView(req *plugin.Request, res *plugin.Response) { + // Defense-in-depth: this route is already gated at the manifest level + // (requirePermissions dashboard.view), but every other project-scope + // entry point re-checks in-handler too (see loadViewWithPanels below), + // so this one shouldn't be the sole exception if the manifest is ever + // misedited. + if !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return + } projectID := req.Caller.ProjectID view, err := p.fetchOrCreateSingletonView(projectID, "project", "", "Dashboard", req.Caller.CallerID) if err != nil { @@ -58,6 +67,14 @@ func (p *dashboardPlugin) getOrCreateAdminView(req *plugin.Request, res *plugin. // visit — same get-or-create-singleton shape as the project/admin scopes, // just keyed by hostViewId instead of projectID/"". func (p *dashboardPlugin) getOrCreateIntegrationView(req *plugin.Request, res *plugin.Response) { + // Defense-in-depth, mirroring getOrCreateProjectView above: this route + // is already gated at the manifest level (requirePermissions + // dashboard.view) so this one shouldn't be the sole exception if the + // manifest is ever misedited. + if !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return + } projectID := req.Caller.ProjectID hostViewID := req.PathParam("hostViewId") if hostViewID == "" { @@ -253,6 +270,24 @@ func (p *dashboardPlugin) loadViewWithPanels(viewID, projectID string, res *plug return nil, false } + // Permission check: views whose dashboard_views.scope is "project" or + // "integration" require dashboard.view/dashboard.manage — a + // project-scope permission pair, checked against the caller's + // per-project permission map. The manifest already enforces this + // uniformly for every route that reaches here (see plugin.json); this + // in-handler check is defense-in-depth so it isn't the sole + // enforcement if the manifest is ever misedited. Rows with + // dashboard_views.scope == "admin" are excluded from this check: they + // only ever reach here via the admin routes (projectID == ""), which + // are already fully gated at the manifest level by dashboard.view/ + // dashboard.manage checked at permission scope "global" — not "admin"; + // the permission system has no such scope, dashboard_views.scope is an + // unrelated data-model value that happens to share the word "admin". + if v.Scope != "admin" && !p.canViewDashboard() { + res.Error(403, "you don't have permission to view this dashboard") + return nil, false + } + panels, err := p.fetchPanelsForView(v.ID) if err != nil { p.log.Error("loadViewWithPanels panels: " + err.Error()) @@ -263,6 +298,16 @@ func (p *dashboardPlugin) loadViewWithPanels(viewID, projectID string, res *plug return &v, true } +// canViewDashboard reports whether the caller may view a project-scope +// dashboard — dashboard.view, or dashboard.manage as a superset (a caller +// who can manage a dashboard can always view it too). Single source of +// truth for that view-or-manage relationship, mirroring canManagePanel's +// shape in panels.go, so the three call sites above stay in sync rather +// than each hand-rolling the same two-permission check. +func (p *dashboardPlugin) canViewDashboard() bool { + return p.perm.Check("dashboard.view") || p.perm.Check("dashboard.manage") +} + func viewFromRow(cols []string, row []any) dashboardView { sc := newRowScanner(cols, row) return dashboardView{ diff --git a/plugin.json b/plugin.json index 1eb0283..5fd2bd4 100644 --- a/plugin.json +++ b/plugin.json @@ -5,6 +5,32 @@ "version": "0.3.4", "minCoreVersion": "v0.13.3", "permissions": ["db.read", "db.write", "events.subscribe", "cache"], + "customPermissions": [ + { + "key": "dashboard.view", + "label": "View dashboard", + "description": "View the project's \"Dashboard\" page and its panels. Does not affect the per-view dashboards embedded in Backlog/Sprint/Timeline pages, which any project member can already see.", + "scope": "project" + }, + { + "key": "dashboard.manage", + "label": "Manage dashboard", + "description": "Create, edit, delete, and rearrange panels on the project's \"Dashboard\" page, and run query previews while authoring them.", + "scope": "project" + }, + { + "key": "dashboard.view", + "label": "View dashboard", + "description": "View the admin-sidebar \"Dashboard\" page — the instance-wide, cross-project dashboard.", + "scope": "global" + }, + { + "key": "dashboard.manage", + "label": "Manage dashboard", + "description": "Create, edit, delete, and rearrange panels on the admin-sidebar \"Dashboard\" page, and run query previews while authoring them.", + "scope": "global" + } + ], "backend": { "eventSubscriptions": [], "routes": [ @@ -14,7 +40,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -23,7 +49,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -32,7 +58,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -41,7 +67,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -50,7 +76,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -59,7 +85,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -68,7 +94,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -77,7 +103,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.read"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -86,7 +112,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "project", "permissions": ["projects.write"] } + { "name": "requirePermissions", "scope": "project", "permissions": ["dashboard.view"] } ] }, { @@ -95,7 +121,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.view"] } ] }, { @@ -104,7 +130,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -113,7 +139,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -122,7 +148,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -131,7 +157,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] }, { @@ -140,7 +166,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.view"] } ] }, { @@ -149,7 +175,7 @@ "middlewares": [ { "name": "optionalAuthn" }, { "name": "requireFreshPassword" }, - { "name": "requirePermissions", "scope": "global", "permissions": ["users.write"] } + { "name": "requirePermissions", "scope": "global", "permissions": ["dashboard.manage"] } ] } ] @@ -181,7 +207,8 @@ "label": "Dashboard", "icon": "LayoutDashboard", "component": "ProjectDashboardPage", - "order": 5 + "order": 5, + "requiredPermission": "dashboard.view" }, { "scope": "admin", @@ -189,7 +216,8 @@ "label": "Dashboard", "icon": "LayoutDashboard", "component": "AdminDashboardPage", - "order": 5 + "order": 5, + "requiredPermission": "dashboard.view" } ] }, From f5338880e611544cdccbbbff51f1b67a2fd7a517 Mon Sep 17 00:00:00 2001 From: pikann22 Date: Mon, 14 Sep 2026 08:35:53 +0000 Subject: [PATCH 2/2] chore: update version to 0.4.0 in plugin.json Minor bump: introduces dashboard.view/dashboard.manage permissions, replacing the projects.read/projects.write/users.write checks these routes used before (existing role grants will need to add them). Co-Authored-By: Claude Sonnet 5 --- plugin.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin.json b/plugin.json index 5fd2bd4..3ee185e 100644 --- a/plugin.json +++ b/plugin.json @@ -2,7 +2,7 @@ "id": "com.paca.dashboard", "displayName": "Dashboard", "description": "Customizable panel-based dashboard builder (charts, tables, text) across the project dashboard page, the admin dashboard page, and a single dashboard per Integration-page view (backlog/sprint/timeline).", - "version": "0.3.4", + "version": "0.4.0", "minCoreVersion": "v0.13.3", "permissions": ["db.read", "db.write", "events.subscribe", "cache"], "customPermissions": [