diff --git a/scripts/audit/no-hardcoded-role-checks.sh b/scripts/audit/no-hardcoded-role-checks.sh index cc66d6fd1..e06c03911 100755 --- a/scripts/audit/no-hardcoded-role-checks.sh +++ b/scripts/audit/no-hardcoded-role-checks.sh @@ -25,7 +25,7 @@ if ! command -v rg >/dev/null 2>&1; then echo "Install with: brew install ripgrep (or: apt-get install ripgrep)" >&2 exit 2 fi -raw=$(rg -n -B1 -A1 '\brole\s*(===|!==)\s*"(admin|technician|member|guest)"' src \ +raw=$(rg -n -B1 -A1 '\b[A-Za-z_$][A-Za-z0-9_$.]*\s*(===|!==)\s*"(admin|technician|member|guest)"' src \ --glob '!src/lib/permissions/matrix.ts' \ --glob '!src/lib/permissions/helpers.ts' \ --glob '!**/*.test.*' \ @@ -62,6 +62,14 @@ matches=$(echo "$raw" | awk ' if (parsed == "") next n = split(parsed, parts, SUBSEP) file = parts[1]; lineno = parts[2] + 0; sep = parts[3]; content = parts[4] + + # Skip pure comment/JSDoc lines — they describe role checks, not perform them. + if (content ~ /^[[:space:]]*(\*|\/\/|\/\*)/) { + if (sep == ":") { + next + } + } + # Record allow markers at this (file, lineno). if (content ~ /permissions-audit-allow:/) { allow[file, lineno] = 1 diff --git a/scripts/tests/test_no_hardcoded_role_checks.py b/scripts/tests/test_no_hardcoded_role_checks.py new file mode 100644 index 000000000..14753c09b --- /dev/null +++ b/scripts/tests/test_no_hardcoded_role_checks.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = PROJECT_ROOT / "scripts/audit/no-hardcoded-role-checks.sh" +pytestmark = pytest.mark.skipif( + shutil.which("rg") is None, + reason="role-audit regression tests require the script's ripgrep prerequisite", +) + + +def run_audit(tmp_path: Path, source: str) -> subprocess.CompletedProcess[str]: + src = tmp_path / "src" + src.mkdir() + (src / "example.ts").write_text(source) + return subprocess.run( + ["bash", str(SCRIPT)], + cwd=tmp_path, + capture_output=True, + text=True, + check=False, + ) + + +def test_detects_arbitrary_identifiers_and_property_access(tmp_path: Path) -> None: + result = run_audit( + tmp_path, + 'if (accessLevel === "admin" || currentUser.role !== "guest") {}\n', + ) + + assert result.returncode == 1 + assert 'accessLevel === "admin"' in result.stderr + + +def test_ignores_role_comparisons_on_pure_comment_lines(tmp_path: Path) -> None: + result = run_audit( + tmp_path, + """// accessLevel === \"admin\" +/* currentUser.role !== \"guest\" */ +/** + * role === \"technician\" + */ +export const harmless = true; +""", + ) + + assert result.returncode == 0 + assert result.stderr == "" + + +def test_accepts_an_adjacent_allow_marker(tmp_path: Path) -> None: + result = run_audit( + tmp_path, + """// permissions-audit-allow: display-only label +if (viewer.role === \"member\") {} +""", + ) + + assert result.returncode == 0 + assert result.stderr == "" diff --git a/src/app/(app)/admin/users/actions.ts b/src/app/(app)/admin/users/actions.ts index dcd4077e5..6a76b2914 100644 --- a/src/app/(app)/admin/users/actions.ts +++ b/src/app/(app)/admin/users/actions.ts @@ -207,7 +207,7 @@ export async function updateUserRole( if ( validated.userType === "active" && validated.userId === user.id && - validated.newRole !== "admin" + validated.newRole !== "admin" // permissions-audit-allow: self-demotion invariant ) { throw new Error("Admins cannot demote themselves"); } diff --git a/src/app/(app)/admin/users/user-role-select.tsx b/src/app/(app)/admin/users/user-role-select.tsx index ae476a519..87eb532cc 100644 --- a/src/app/(app)/admin/users/user-role-select.tsx +++ b/src/app/(app)/admin/users/user-role-select.tsx @@ -34,7 +34,7 @@ export function UserRoleSelect({ if ( userType === "active" && userId === currentUserId && - newRole !== "admin" + newRole !== "admin" // permissions-audit-allow: self-demotion invariant ) { toast.error("You cannot demote yourself."); return; @@ -58,7 +58,7 @@ export function UserRoleSelect({ defaultValue={currentRole} onValueChange={handleRoleChange} disabled={ - isPending || (userId === currentUserId && currentRole === "admin") + isPending || (userId === currentUserId && currentRole === "admin") // permissions-audit-allow: self-demotion invariant } > needs_service > operational * * Accessible to all users (unauthenticated, guest, member, admin). - * The "Add Machine" button is only shown to admins. + * The "Add Machine" button follows the machines.create permission. */ export default async function MachinesPage({ searchParams, @@ -150,19 +150,20 @@ export default async function MachinesPage({ filters.sort ?? "name_asc" ); - const addMachineButton = - accessLevel === "admin" || accessLevel === "technician" ? ( - - ) : undefined; + const canCreateMachine = checkPermission("machines.create", accessLevel); + + const addMachineButton = canCreateMachine ? ( + + ) : undefined; return ( @@ -189,12 +190,12 @@ export default async function MachinesPage({ icon={Plus} title="No machines yet" description={ - accessLevel === "admin" || accessLevel === "technician" + canCreateMachine ? "Get started by adding your first machine to the collection." : "No machines have been added to the collection yet." } action={ - accessLevel === "admin" || accessLevel === "technician" ? ( + canCreateMachine ? (