Skip to content

SKILL.md

Latest commit

 

History

History
225 lines (167 loc) · 10.7 KB

SKILL.md

File metadata and controls

225 lines (167 loc) · 10.7 KB
Raw
name nextcloud-agents-review
description Use when reviewing Nextcloud app code, validating before deployment, or checking for common mistakes. Prevents deploying code with missing security attributes, incorrect DI patterns, and known anti-patterns. Covers controller security attributes, OCS endpoint patterns, database migration integrity, DI patterns, frontend import paths, CSRF handling, file API usage, and known anti-patterns. Keywords: code review, validation, security attributes, anti-pattern, DI check, migration check, CSRF check, deployment, check my code, review before deploy, find mistakes, validate app quality..
license MIT
compatibility Designed for Claude Code. Requires Nextcloud 28+.
metadata
author version
OpenAEC-Foundation
1.0

nextcloud-agents-review

Purpose

This skill guides Claude through a systematic code review of Nextcloud app code. Run this checklist BEFORE merging, deploying, or accepting generated code. Each check specifies WHAT to verify, the EXPECTED state, and the COMMON failure.

Review Process

ALWAYS follow this sequence:

  1. Gather scope -- Identify all files changed or generated
  2. Run area checklists -- Apply each relevant section below
  3. Report findings -- List every violation with file, line, and fix
  4. Block on critical -- NEVER approve code with critical violations

Area 1: Controller Security Attributes

ALWAYS verify every public controller method has explicit security attributes.

Check Expected State Common Failure
Security attributes present Every public method has at least one of #[NoAdminRequired], #[PublicPage], or is intentionally admin-only Missing attributes -- method silently defaults to admin-only
No unprotected state-changing endpoints #[PublicPage] + #[NoCSRFRequired] NEVER appear together on POST/PUT/DELETE methods CSRF bypass on public write endpoints
Brute force protection on auth endpoints #[BruteForceProtection] present on login/token/password methods Missing throttle on authentication
Throttle only on failure $response->throttle() called ONLY in failure branches Throttle on success degrades legitimate users
Rate limiting on sensitive endpoints #[UserRateLimit] or #[AnonRateLimit] on expensive operations Unlimited calls to resource-heavy endpoints

Decision tree:

Method is public?
├── YES: Has security attributes?
│   ├── NO → CRITICAL: Add explicit attributes
│   └── YES: Is state-changing (POST/PUT/DELETE)?
│       ├── YES: Has #[PublicPage] + #[NoCSRFRequired]?
│       │   ├── YES → CRITICAL: Remove one or add alternative auth
│       │   └── NO → OK
│       └── NO → OK
└── NO → SKIP (private/protected methods)

Area 2: OCS Endpoint Validation

ALWAYS verify OCS controllers and routes follow the v2 pattern.

Check Expected State Common Failure
Extends OCSController OCS route controllers extend OCSController, NOT Controller Using Controller breaks response envelope
Routes in ocs array OCS endpoints listed in the ocs array of routes.php Listed in routes array -- wrong URL prefix
Returns DataResponse OCS methods return DataResponse, not JSONResponse JSONResponse bypasses OCS envelope
OCS-APIRequest header API documentation/tests include OCS-APIRequest: true Requests rejected without header
v2 endpoints only All OCS URLs use /ocs/v2.php/ prefix Using v1 with wrong status code assumptions

Area 3: Database Migration Integrity

ALWAYS verify migrations follow Nextcloud conventions.

Check Expected State Common Failure
Naming convention Class: Version{MajorMinor}Date{Timestamp} Wrong naming breaks migration order
No modified migrations Existing migration files NEVER changed Modified migration skipped on instances that already ran it
Primary key exists Every table has setPrimaryKey(['id']) Galera Cluster replication fails
Auto-increment id Every table has id BIGINT autoincrement Missing primary key column
Table name length Table name max 23 chars (27 with oc_ prefix) Oracle compatibility failure
Column name length Column/index names max 30 chars Oracle compatibility failure
Uses query builder No raw SQL strings in migrations postSchemaChange Database portability broken
Index names explicit All indices have named identifiers Cannot reference index for future modification

Decision tree:

Is this a schema change?
├── Existing migration file modified?
│   └── YES → CRITICAL: Create new migration instead
├── New table without primary key?
│   └── YES → CRITICAL: Add setPrimaryKey(['id'])
├── Table name > 23 chars?
│   └── YES → CRITICAL: Shorten name
└── All checks pass → OK

Area 4: Dependency Injection

ALWAYS verify services use constructor injection.

Check Expected State Common Failure
No Server::get() Zero occurrences of \OCP\Server::get() Service locator breaks testability
Constructor injection All dependencies passed via constructor Runtime resolution via container
No ILogger Zero imports of OCP\ILogger Deprecated since NC 24 -- use Psr\Log\LoggerInterface
Namespace in info.xml <namespace> element present Auto-wiring silently fails
No service queries in register() register() only calls IRegistrationContext methods Queries fail -- other apps not yet registered
No side effects in constructors Constructors only assign dependencies I/O in constructor breaks lazy loading

Area 5: Frontend Validation

ALWAYS verify frontend code uses @nextcloud/* packages.

Check Expected State Common Failure
No OC.generateUrl() Zero uses of OC.generateUrl Legacy global -- use @nextcloud/router
No OC.requestToken Zero direct uses of OC.requestToken Use @nextcloud/axios which handles CSRF
No raw fetch/axios Zero imports of plain axios or fetch() Missing auth headers -- use @nextcloud/axios
@nextcloud/vue imports Components from @nextcloud/vue, not custom UI Inconsistent look, no dark mode support
No hardcoded colors CSS uses var(--color-*) variables Breaks dark mode and custom themes
Dialog styles imported @nextcloud/dialogs/style.css imported when using toasts Toasts render without styling
loadState has fallback loadState() calls include third argument Throws on missing key without fallback

Area 6: File API Validation

ALWAYS verify file operations use the Node API correctly.

Check Expected State Common Failure
NotFoundException handled Every get() call wrapped in try/catch for NotFoundException Unhandled exception crashes request
getById() array check getById() result checked as array before access Assumes single result -- returns array
No hardcoded paths File access via getUserFolder(), not string paths Bypasses storage abstraction
IRootFolder injected IRootFolder as constructor dependency, not IUserFolder Cannot access other users' files when needed
instanceof check get() result checked with instanceof File or instanceof Folder Type confusion between files and folders

Area 7: Event System Validation

ALWAYS verify events use the modern typed system.

Check Expected State Common Failure
Typed events only All events extend OCP\EventDispatcher\Event Using deprecated GenericEvent
No deprecated hooks Zero uses of $manager->listen() Legacy hook system -- use IEventDispatcher
instanceof check in handle() handle() method checks $event instanceof SpecificEvent Base Event type causes runtime errors
Listeners in register() Listeners registered via IRegistrationContext::registerEventListener() Registered in boot() -- may miss early events
parent::__construct() called Custom event constructors call parent::__construct() Event base class not initialized
No dispatch in constructors Events dispatched in service methods, not constructors Services may not be fully initialized

Area 8: Security Cross-Checks

ALWAYS run these cross-cutting security checks.

Check Expected State Common Failure
CSRF on state change POST/PUT/DELETE endpoints have CSRF protection OR OCS-APIRequest State changes without CSRF protection
No PublicPage on admin endpoints Admin-settings controllers have NO #[PublicPage] Settings exposed to anonymous users
Password not logged No $password or credential values in logger calls Credentials in log files
SQL injection prevention All query builder params use createNamedParameter() Raw string concatenation in queries
Input validation Controller params have type hints Untyped string injection

Severity Levels

Level Action Examples
CRITICAL MUST fix before merge Missing CSRF, modified migration, no primary key, PublicPage+NoCSRFRequired on POST
WARNING SHOULD fix before merge Missing rate limiting, no NotFoundException handling, deprecated ILogger
INFO Consider fixing Missing explicit security attributes on admin-only method, barrel imports

Review Report Template

## Nextcloud Code Review

### Summary
- Files reviewed: {count}
- Critical: {count}
- Warnings: {count}
- Info: {count}

### Critical Issues
- [{file}:{line}] {area}: {description} → Fix: {action}

### Warnings
- [{file}:{line}] {area}: {description} → Fix: {action}

### Info
- [{file}:{line}] {area}: {description} → Fix: {action}

### Verdict: {PASS | BLOCK}

Reference Links

Official Sources