nextcloud-agents-review

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..

Jump to install

Repository: Impertio-Studio/Nextcloud-Claude-Skill-Package
Last source activity: March 30, 2026 at 18:20
Detected SKILL.md language: English
Stars: 2
Forks: 0

Install options

The review-first prompt is selected by default. You can switch to a direct command or download a local copy.

Review the source files

Read SKILL.md and any companion files shown by SkillsMP before deciding whether to install.

Back to search Back to installation

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

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)

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

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

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

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

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

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

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

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

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

## 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}