Enforce the permission-name rule at discovery, for every plugin
CI / full-gate (push) Failing after 1m53s
CI / full-gate (push) Failing after 1m53s
This commit is contained in:
@@ -50,8 +50,13 @@ const badCases: Array<{ name: string; files: Record<string, string>; match: RegE
|
||||
{ name: "non-function dashboard", files: { "weirddash/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: "nope" };` }, match: /weirddash.*dashboard.*function/s },
|
||||
{ name: "reserved dashboard id shadows the gated dashboard", files: { "dashboard/plugin.ts": full("dashboard") }, match: /dashboard.*reserved/s },
|
||||
{ name: "duplicate nav id across plugins", files: { "a/plugin.ts": full("a").replace("a:root", "dup"), "b/plugin.ts": full("b").replace("b:root", "dup") }, match: /nav id "dup"/ },
|
||||
{ name: "a route marked public AND permission is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", public: true, permission: "x", handler: () => ({ html: "x" }) }] };` }, match: /contra.*public.*permission/s },
|
||||
{ name: "a nav node marked public AND permission is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "1.0.0", nav: [{ id: "n", label: "N", public: true, permission: "x" }] };` }, match: /contranav.*public.*permission/s },
|
||||
{ name: "a route marked public AND permission is contradictory", files: { "contra/plugin.ts": `export default { apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", public: true, permission: "x:read", handler: () => ({ html: "x" }) }] };` }, match: /contra.*public.*permission/s },
|
||||
{ name: "a nav node marked public AND permission is contradictory", files: { "contranav/plugin.ts": `export default { apiVersion: "1.0.0", nav: [{ id: "n", label: "N", public: true, permission: "x:read" }] };` }, match: /contranav.*public.*permission/s },
|
||||
// A permission name is <resource>:<action> wherever the manifest mentions one. Enforced here, not
|
||||
// only in the admin GUI, so it holds for a plugin installed without that GUI.
|
||||
{ name: "a route gating on a bare word", files: { "bare/plugin.ts": `export default { apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", permission: "admin", handler: () => ({ html: "x" }) }] };` }, match: /bare.*admin.*<resource>:<action>/s },
|
||||
{ name: "a nav node gating on a bare word", files: { "barenav/plugin.ts": `export default { apiVersion: "1.0.0", nav: [{ id: "n", label: "N", permission: "admin" }] };` }, match: /barenav.*admin.*<resource>:<action>/s },
|
||||
{ name: "a declared permission that is a bare word", files: { "baredecl/plugin.ts": `export default { apiVersion: "1.0.0", permissions: [{ name: "admin" }] };` }, match: /baredecl.*admin.*<resource>:<action>/s },
|
||||
{ name: "two plugins claim the public home", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "b" }) };` }, match: /home/ },
|
||||
{ name: "two plugins claim the gated dashboard", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "b" }) };` }, match: /dashboard/ },
|
||||
];
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
import { existsSync, readdirSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { checkApiVersion, findConflicts, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts";
|
||||
import { checkApiVersion, findConflicts, isValidPermissionName, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts";
|
||||
|
||||
const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..");
|
||||
|
||||
@@ -100,6 +100,20 @@ function shapeError(manifest: PluginManifest): string | null {
|
||||
}
|
||||
const navContradiction = findPublicNavContradiction(manifest.nav);
|
||||
if (navContradiction) return navContradiction;
|
||||
// Every permission name the manifest mentions — gated on or declared — must be `<resource>:<action>`.
|
||||
// A bare word names a role, and roles are groups here (README → Naming a permission).
|
||||
for (const route of Array.isArray(manifest.routes) ? manifest.routes : []) {
|
||||
if (route?.permission != null && !isValidPermissionName(route.permission)) {
|
||||
return `route "${route.method} ${route.path}" gates on "${route.permission}"; a permission name is <resource>:<action>, e.g. "things:read"`;
|
||||
}
|
||||
}
|
||||
for (const decl of Array.isArray(manifest.permissions) ? manifest.permissions : []) {
|
||||
if (decl?.name == null || !isValidPermissionName(decl.name)) {
|
||||
return `declared permission "${decl?.name}" is not <resource>:<action>, e.g. "things:read"`;
|
||||
}
|
||||
}
|
||||
const navPermission = findInvalidNavPermission(manifest.nav);
|
||||
if (navPermission) return navPermission;
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -113,6 +127,17 @@ function findPublicNavContradiction(nodes: PluginManifest["nav"]): string | null
|
||||
return null;
|
||||
}
|
||||
|
||||
function findInvalidNavPermission(nodes: PluginManifest["nav"]): string | null {
|
||||
for (const node of Array.isArray(nodes) ? nodes : []) {
|
||||
if (node?.permission != null && !isValidPermissionName(node.permission)) {
|
||||
return `nav node "${node.label ?? node.id ?? "?"}" gates on "${node.permission}"; a permission name is <resource>:<action>, e.g. "things:read"`;
|
||||
}
|
||||
const inChild = findInvalidNavPermission(node?.children);
|
||||
if (inChild) return inChild;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function messageOf(err: unknown): string {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
// contract boundary in code — the host may refactor any other src/* freely as long as it holds, so
|
||||
// a plugin should import from here, never reach into deeper modules. See README.md → Building plugins.
|
||||
|
||||
export { definePlugin } from "./plugin.ts";
|
||||
export { definePlugin, isValidPermissionName } from "./plugin.ts";
|
||||
export type { HttpMethod, Plugin, PluginHooks, PluginManifest, PermissionDecl, Route, RouteHandler, RouteResult } from "./plugin.ts";
|
||||
export type { RequestContext, User } from "../http/context.ts";
|
||||
export type { PageChrome } from "../ui/chrome.ts";
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
definePlugin,
|
||||
findConflicts,
|
||||
HOST_API_VERSION,
|
||||
isValidPermissionName,
|
||||
isValidPluginId,
|
||||
parseSemver,
|
||||
RESERVED_PLUGIN_IDS,
|
||||
@@ -47,6 +48,16 @@ test("isValidPluginId accepts lowercase/digits/dashes anywhere and rejects every
|
||||
}
|
||||
});
|
||||
|
||||
test("isValidPermissionName requires <resource>:<action> — a bare word names a role, and roles are groups", () => {
|
||||
for (const ok of ["users:read", "scheduling:write", "oauth2-clients:read", "team-a:a1_b9", "invoices:approve"]) {
|
||||
assert.ok(isValidPermissionName(ok), ok);
|
||||
}
|
||||
// "admin" is the shape this rule exists to stop: it says who someone is, not what they may do.
|
||||
for (const bad of ["admin", "", "Users:read", "users:", ":read", "users:read:extra", "a b:read", "-bad:read", "a/b:read", `${"a".repeat(60)}:read`]) {
|
||||
assert.ok(!isValidPermissionName(bad), bad);
|
||||
}
|
||||
});
|
||||
|
||||
test("parseSemver follows the semver core, rejecting ranges, prefixes, leading zeros and missing parts", () => {
|
||||
assert.deepEqual(parseSemver("1.2.3"), { major: 1, minor: 2, patch: 3 });
|
||||
assert.deepEqual(parseSemver("1.2.3-rc.1+build.5"), { major: 1, minor: 2, patch: 3 }); // prerelease/build tolerated, ignored
|
||||
|
||||
@@ -45,6 +45,15 @@ export interface PermissionDecl {
|
||||
name: string;
|
||||
}
|
||||
|
||||
// `<resource>:<action>`, each half lowercase alphanumeric with dashes/underscores inside. The 64-char
|
||||
// cap keeps a name usable as a Keto object and a URL path segment. Enforced at discovery like every
|
||||
// other manifest rule, so the convention holds for plugins the admin GUI never touches.
|
||||
const PERMISSION_NAME = /^[a-z0-9][a-z0-9_-]*:[a-z0-9][a-z0-9_-]*$/;
|
||||
|
||||
export function isValidPermissionName(name: string): boolean {
|
||||
return name.length <= 64 && PERMISSION_NAME.test(name);
|
||||
}
|
||||
|
||||
// Optional hooks on system actions. Crash-isolation is a non-goal — a throwing hook fails loud.
|
||||
export interface PluginHooks {
|
||||
onBoot?: () => Promise<void> | void; // after discovery, before the server listens
|
||||
|
||||
Reference in New Issue
Block a user