diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..5bb52e8 --- /dev/null +++ b/TODO.md @@ -0,0 +1,174 @@ +# TODO.md - core-developer + +Last reviewed: 2026-01-29 + +## P1 - Critical / Security + +### Completed +- [x] **Server model has no migration** - FIXED: Migration created at `src/Migrations/0001_01_01_000001_create_developer_tables.php` +- [x] **Inconsistent Hades authorization in DevController** - FIXED: Now uses `$user->isHades()` method +- [x] **SetHadesCookie uses env() directly** - FIXED: Now uses `config('developer.hades_token')` +- [x] **HorizonServiceProvider gate is empty** - FIXED: `viewHorizon` gate now checks `$user->isHades()` +- [x] **TelescopeServiceProvider gate emails empty** - FIXED: Telescope gate now checks `$user->isHades()` + +### Open + +_No open P1 items._ + +### Recently Fixed (Jan 2026) + +- [x] **Database component SQL injection hardening** - FIXED: `isReadOnlyQuery()` now blocks stacked queries by checking for semicolons followed by non-whitespace content using regex `/;\s*\S/`. + - File: `src/View/Modal/Admin/Database.php` + +- [x] **DevController missing strict types declaration** - FIXED: Added `declare(strict_types=1);` at top of file. + - File: `src/Controllers/DevController.php` + +- [x] **Servers component writes private key to temp file** - FIXED: Now uses `tempnam()` for atomic file creation and sets 0600 permissions before writing sensitive data. + - File: `src/View/Modal/Admin/Servers.php` + +- [x] **CopyDeviceFrames command lacks config validation** - FIXED: Added validation for config existence and required keys (source_path, public_path, devices) with proper error messages. + - File: `src/Console/Commands/CopyDeviceFrames.php` + +## P2 - High Priority + +### Completed +- [x] **Unify authorization pattern** - Created `RequireHades` middleware for consistent authorization +- [x] **Add rate limiting to API routes** - Added rate limiters in `Boot.php` for all endpoints +- [x] **Log clear action should be audited** - `clearLogs()` now logs with user_id, email, previous_size, IP +- [x] **Remove duplicate log reading logic** - Created `LogReaderService` used by both DevController and Logs component +- [x] **RemoteServerManager timeout is hardcoded** - Added configurable timeouts via config + +### Open + +- [ ] **Tests pass without Hades tier set** - Tests in `DevToolsBasic.php` create a user without Hades tier but tests pass, suggesting authorization may not be enforced correctly in test environment + - File: `src/Tests/UseCase/DevToolsBasic.php` + - Acceptance: Tests should fail when user is not Hades; add trait/helper to set Hades status in tests + +- [ ] **Clear logs button has no confirmation** - Unlike cache management, the logs clear button executes immediately without confirmation + - File: `src/View/Modal/Admin/Logs.php` and corresponding blade + - Acceptance: Add confirmation modal similar to Cache component + +- [ ] **Activity log component shows all workspaces** - ActivityLog queries all Activity records without workspace scoping + - File: `src/View/Modal/Admin/ActivityLog.php` + - Acceptance: Either scope to current workspace or document that this is intentional for Hades users + +### Recently Fixed (Jan 2026) + +- [x] **Missing developer config file** - FIXED: Config file exists at `src/config.php` with hades_token, ssh.connection_timeout, ssh.command_timeout, and horizon notification settings. Published via `mergeConfigFrom()` in Boot.php. + +- [x] **Livewire pages have no route middleware** - FIXED: RequireHades middleware applied to the `/hub/dev/*` route group in `src/Routes/admin.php` line 15. Authorization now enforced at route level. + +## P3 - Medium Priority + +### Completed +- [x] **Multi-log file support** - Added `getAvailableLogFiles()` and `getCurrentLogPath()` to LogReaderService +- [x] **Command registration** - CopyDeviceFrames now registered via `onConsole()` handler + +### Open + +- [ ] **Server model missing table name specification** - Relies on Laravel convention; should explicitly set `$table = 'servers'` + - File: `src/Models/Server.php` + - Acceptance: Add protected `$table` property + +- [ ] **LogReaderService redaction patterns need review** - IP redaction pattern may miss IPv6 addresses + - File: `src/Services/LogReaderService.php` line 42 + - Acceptance: Add IPv6 support or document limitation + +- [ ] **RouteTestService environment check is permissive** - `isTestingAllowed()` returns true for 'testing' environment which could be used in CI + - File: `src/Services/RouteTestService.php` line 47 + - Acceptance: Consider adding config flag to explicitly enable route testing + +- [ ] **Database query tool lacks export functionality** - Users can view results but cannot download/export them + - Acceptance: Add CSV/JSON export button for query results + +- [ ] **Route inspector history not persisted** - History is lost on page refresh + - File: `src/View/Modal/Admin/RouteInspector.php` + - Acceptance: Consider storing history in session or localStorage + +- [ ] **Missing translations for some UI elements** - Servers, Database, and ActivityLog pages have hardcoded English strings instead of using translation keys + - Files: Multiple blade files and components + - Acceptance: Add translation keys to `src/Lang/en_GB/developer.php` and use them consistently + +## P4 - Low Priority / Improvements + +### Open + +- [ ] **DevController has redundant authorize() calls** - The `routes()` and `session()` methods call `$this->authorize()` but API routes already have `RequireHades` middleware + - File: `src/Controllers/DevController.php` + - Acceptance: Remove redundant authorization checks or document the defence-in-depth approach + +- [ ] **LogReaderService could use generators** - For very large log files, using generators instead of arrays would reduce memory usage + - File: `src/Services/LogReaderService.php` + - Acceptance: Refactor `readLogEntries()` to optionally yield entries + +- [ ] **RouteTestResult getFormattedResponseTime has edge case** - Times under 1ms are converted to microseconds incorrectly (multiplied by 1000 instead of keeping as sub-millisecond) + - File: `src/Data/RouteTestResult.php` line 93 + - Acceptance: Fix calculation or clarify the intended behaviour + +- [ ] **Server status enum should be a proper PHP enum** - Currently uses string values ('pending', 'connected', 'failed') + - File: `src/Models/Server.php` + - Acceptance: Create `ServerStatus` backed enum and use it consistently + +- [ ] **ApplyIconSettings middleware has hardcoded defaults** - Default values should come from config + - File: `src/Middleware/ApplyIconSettings.php` + - Acceptance: Move defaults to config file + +- [ ] **Pulse dashboard override lacks documentation** - The custom Pulse view is registered but not documented + - File: `src/View/Blade/vendor/pulse/dashboard.blade.php` + - Acceptance: Add comment in Boot.php explaining the override purpose + +## P5 - Nice to Have / Future + +### From code-review.md (documented features) +- [ ] **Server CRUD UI improvements** - Add bulk actions, SSH key validation, connection health checks +- [ ] **Log download/export** - FIXED: `downloadLogs()` added, but could add format options (JSON, filtered export) +- [ ] **Event log viewer** - Activity logs exist on Server model but no dedicated UI to view activity per model + +### New Ideas +- [ ] **Add log search functionality** - Currently only filter by level, add full-text search within log messages +- [ ] **Database saved queries** - Allow saving frequently used queries with names +- [ ] **Route documentation viewer** - Parse DocBlocks from controllers and display in route inspector +- [ ] **SSH terminal emulator** - Interactive terminal for connected servers (complex, security considerations) +- [ ] **Cache statistics dashboard** - Show cache hit/miss rates, memory usage, key counts +- [ ] **Config diff viewer** - Compare current config values against defaults + +## P6+ - Backlog / Someday + +- [ ] **Real-time log streaming** - WebSocket/SSE for live log tailing +- [ ] **Query explain plan visualisation** - Parse and display EXPLAIN output graphically +- [ ] **Route performance profiling** - Track response times over time, identify slow routes +- [ ] **Deployment integration** - Trigger deployments from server management UI +- [ ] **Multi-database support** - Query tool for multiple database connections +- [ ] **Scheduled task monitoring** - View and manage Laravel scheduled tasks + +## Test Coverage Gaps + +### Currently Tested +- Logs page renders with correct sections and translations +- Routes page renders with table headers +- Cache page renders with all cache action cards + +### Missing Tests +- [ ] DevController API endpoints (logs, routes, session, clear) +- [ ] Cache clearing actually executes and clears caches +- [ ] Log filtering by level +- [ ] Route filtering/searching +- [ ] Hades authorization enforcement (both allow and deny cases) +- [ ] RemoteServerManager SSH operations (mock phpseclib) +- [ ] Server model scopes and methods +- [ ] SetHadesCookie listener +- [ ] ApplyIconSettings middleware +- [ ] CopyDeviceFrames command +- [ ] LogReaderService redaction patterns +- [ ] RouteTestService route testing logic +- [ ] Database component query execution and validation +- [ ] ActivityLog filtering and pagination +- [ ] RouteInspector request building and execution + +## Notes + +- All Hades-only features require the user's `isHades()` method to return true +- The module depends on `host-uk/core` and `host-uk/core-admin` +- UK English spellings must be used (colour, organisation, centre) +- All PHP files should have `declare(strict_types=1);` +- Testing uses Pest syntax, not PHPUnit diff --git a/src/Boot.php b/src/Boot.php index c4edf46..2166d12 100644 --- a/src/Boot.php +++ b/src/Boot.php @@ -31,6 +31,7 @@ class Boot extends ServiceProvider implements AdminMenuProvider public function boot(): void { $this->loadTranslationsFrom(__DIR__.'/Lang', 'developer'); + $this->mergeConfigFrom(__DIR__.'/config.php', 'developer'); app(AdminMenuRegistry::class)->register($this); diff --git a/src/Console/Commands/CopyDeviceFrames.php b/src/Console/Commands/CopyDeviceFrames.php index d69fcf6..c29c581 100644 --- a/src/Console/Commands/CopyDeviceFrames.php +++ b/src/Console/Commands/CopyDeviceFrames.php @@ -1,5 +1,7 @@ error('The device-frames configuration is not defined. Please publish the config file.'); + + return Command::FAILURE; + } + + $requiredKeys = ['source_path', 'public_path', 'devices']; + $missingKeys = array_diff($requiredKeys, array_keys($config)); + if (! empty($missingKeys)) { + $this->error('Missing required config keys: '.implode(', ', $missingKeys)); + + return Command::FAILURE; + } + + if (! is_array($config['devices']) || empty($config['devices'])) { + $this->error('The device-frames.devices configuration must be a non-empty array.'); + + return Command::FAILURE; + } + $sourcePath = $config['source_path']; $publicPath = public_path($config['public_path']); diff --git a/src/Controllers/DevController.php b/src/Controllers/DevController.php index a36d293..fd21c87 100644 --- a/src/Controllers/DevController.php +++ b/src/Controllers/DevController.php @@ -1,5 +1,7 @@ name('hub.')->group(function () { - // Developer tools (Hades only) - authorization checked in components - Route::prefix('dev')->name('dev.')->group(function () { - Route::get('/logs', \Core\Developer\View\Modal\Admin\Logs::class)->name('logs'); - Route::get('/routes', \Core\Developer\View\Modal\Admin\Routes::class)->name('routes'); - Route::get('/cache', \Core\Developer\View\Modal\Admin\Cache::class)->name('cache'); - Route::get('/activity', \Core\Developer\View\Modal\Admin\ActivityLog::class)->name('activity'); - Route::get('/servers', \Core\Developer\View\Modal\Admin\Servers::class)->name('servers'); - Route::get('/database', \Core\Developer\View\Modal\Admin\Database::class)->name('database'); - Route::get('/route-inspector', \Core\Developer\View\Modal\Admin\RouteInspector::class)->name('route-inspector'); - }); + // Developer tools (Hades only) - authorization enforced via middleware + Route::prefix('dev') + ->name('dev.') + ->middleware(\Core\Developer\Middleware\RequireHades::class) + ->group(function () { + Route::get('/logs', \Core\Developer\View\Modal\Admin\Logs::class)->name('logs'); + Route::get('/routes', \Core\Developer\View\Modal\Admin\Routes::class)->name('routes'); + Route::get('/cache', \Core\Developer\View\Modal\Admin\Cache::class)->name('cache'); + Route::get('/activity', \Core\Developer\View\Modal\Admin\ActivityLog::class)->name('activity'); + Route::get('/servers', \Core\Developer\View\Modal\Admin\Servers::class)->name('servers'); + Route::get('/database', \Core\Developer\View\Modal\Admin\Database::class)->name('database'); + Route::get('/route-inspector', \Core\Developer\View\Modal\Admin\RouteInspector::class)->name('route-inspector'); + }); }); /* diff --git a/src/View/Modal/Admin/Database.php b/src/View/Modal/Admin/Database.php index 4bd5a14..9b9366a 100644 --- a/src/View/Modal/Admin/Database.php +++ b/src/View/Modal/Admin/Database.php @@ -136,7 +136,17 @@ class Database extends Component // Get first word of query $firstWord = strtoupper(strtok($query, ' ')); - return in_array($firstWord, self::ALLOWED_STATEMENTS, true); + if (! in_array($firstWord, self::ALLOWED_STATEMENTS, true)) { + return false; + } + + // Block stacked queries (e.g., "SELECT 1; DROP TABLE users") + // Check for semicolon followed by any non-whitespace content + if (preg_match('/;\s*\S/', $query)) { + return false; + } + + return true; } private function checkHadesAccess(): void diff --git a/src/View/Modal/Admin/Servers.php b/src/View/Modal/Admin/Servers.php index 5f6f46e..b06644f 100644 --- a/src/View/Modal/Admin/Servers.php +++ b/src/View/Modal/Admin/Servers.php @@ -165,10 +165,16 @@ class Servers extends Component } try { - // Create a temporary key file - $tempKeyPath = sys_get_temp_dir().'/ssh_test_'.uniqid(); - file_put_contents($tempKeyPath, $server->getDecryptedPrivateKey()); + // Create a unique temporary key file with secure permissions + // Using tempnam() for atomic file creation to prevent race conditions + $tempKeyPath = tempnam(sys_get_temp_dir(), 'ssh_key_'); + if ($tempKeyPath === false) { + throw new \RuntimeException('Failed to create temporary key file'); + } + + // Set restrictive permissions before writing sensitive data chmod($tempKeyPath, 0600); + file_put_contents($tempKeyPath, $server->getDecryptedPrivateKey()); // Test SSH connection with a simple echo command $result = Process::timeout(15)->run([ diff --git a/src/config.php b/src/config.php new file mode 100644 index 0000000..5954206 --- /dev/null +++ b/src/config.php @@ -0,0 +1,68 @@ + env('HADES_TOKEN'), + + /* + |-------------------------------------------------------------------------- + | SSH Configuration + |-------------------------------------------------------------------------- + | + | Settings for SSH connections to remote servers via the RemoteServerManager + | trait. These timeouts help prevent hanging connections and runaway commands. + | + */ + + 'ssh' => [ + // Connection timeout in seconds (time to establish SSH connection) + 'connection_timeout' => env('DEVELOPER_SSH_CONNECTION_TIMEOUT', 30), + + // Command timeout in seconds (max time for individual commands) + 'command_timeout' => env('DEVELOPER_SSH_COMMAND_TIMEOUT', 60), + ], + + /* + |-------------------------------------------------------------------------- + | Horizon Notifications + |-------------------------------------------------------------------------- + | + | Configure notification channels for Laravel Horizon alerts. + | These notifications are sent when long wait times or job failures occur. + | + */ + + 'horizon' => [ + // SMS notification recipient (phone number) + 'sms_to' => env('HORIZON_SMS_TO'), + + // Email notification recipient + 'mail_to' => env('HORIZON_MAIL_TO'), + + // Slack webhook URL for notifications + 'slack_webhook' => env('HORIZON_SLACK_WEBHOOK'), + + // Slack channel for notifications (default: #alerts) + 'slack_channel' => env('HORIZON_SLACK_CHANNEL', '#alerts'), + ], + +];