php-uptelligence/TODO.md
Snider 6f71edd14e fix(security): address P2 security items and migration mismatch
P2-058: Migration Mismatch
- Created new migration for vendor tracking tables (000004)
- Added explicit $table property to all models with uptelligence_ prefix
- Clarified dual-purpose nature (uptime monitoring + vendor tracking)
- Added appropriate indexes for common query patterns

P2-059: Webhook Signature Timing Attack Audit
- Verified all signature verification uses hash_equals()
- Added comprehensive tests in WebhookSignatureVerificationTest.php
- Tests cover all providers, grace periods, edge cases

P2-060: API Key Exposure in Logs
- Added redactSensitiveData() to AIAnalyzerService
- Added redactSensitiveData() to IssueGeneratorService
- Added redactSensitiveData() to VendorUpdateCheckerService
- Redacts API keys, tokens, bearer tokens, auth headers

P2-061: Missing Webhook Payload Validation
- Added MAX_PAYLOAD_SIZE (1MB) and MAX_JSON_DEPTH (32) limits
- Added validatePayloadSize() for DoS protection
- Added parseAndValidateJson() with depth limit
- Added validatePayloadStructure() for provider-specific validation
- Added hasExcessiveArraySize() to prevent memory exhaustion
- Added tests in WebhookPayloadValidationTest.php

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-29 13:29:26 +00:00

14 KiB

TODO - core-uptelligence

Upstream vendor tracking and dependency intelligence for Host UK.

Previous review: See changelog/2026/jan/code-review.md for historical context. Many issues were fixed in the 2026-01-21 wave.

P1 - Critical / Security

Migration Mismatch - Uptime Monitoring vs Vendor Tracking FIXED (P2-058)

FIXED: 2026-01-29

The package now clarifies that it serves dual purposes:

  1. Uptime monitoring (existing migration 000001) - for server health tracking
  2. Vendor tracking (new migration 000004) - for upstream dependency intelligence

Changes made:

  • Created new migration 0001_01_01_000004_create_uptelligence_vendor_tables.php with all vendor tracking tables
  • Added explicit $table property to all models with uptelligence_ prefix:
    • Vendor -> uptelligence_vendors
    • VersionRelease -> uptelligence_version_releases
    • UpstreamTodo -> uptelligence_upstream_todos
    • DiffCache -> uptelligence_diff_cache
    • AnalysisLog -> uptelligence_analysis_logs
    • Asset -> uptelligence_assets
    • AssetVersion -> uptelligence_asset_versions
  • Added appropriate indexes for common query patterns
  • Documented dual-purpose nature in migration comments

Webhook Signature Timing Attack Vulnerability FIXED (P2-059)

FIXED: 2026-01-29

Audit result: All signature verification methods already use hash_equals() for timing-safe comparison. The implementation is correct.

Changes made:

  • Audited all signature verification paths - all use hash_equals()
  • Added comprehensive unit tests in tests/Unit/WebhookSignatureVerificationTest.php:
    • Tests for all providers (GitHub, GitLab, npm, Packagist, custom)
    • Tests for grace period/secret rotation
    • Tests for malformed signatures
    • Tests for binary payloads
    • Tests for edge cases (empty payloads, large payloads)

API Key Exposure in Logs FIXED (P2-060)

FIXED: 2026-01-29

Changes made:

  • Added redactSensitiveData() method to AIAnalyzerService
  • Added redactSensitiveData() method to IssueGeneratorService
  • Added redactSensitiveData() method to VendorUpdateCheckerService
  • All Log::error calls now pass through redaction before logging
  • Redaction patterns cover:
    • Anthropic API keys (sk-ant-...)
    • OpenAI API keys (sk-...)
    • GitHub tokens (ghp_..., gho_..., github_pat_...)
    • Bearer tokens
    • Authorization headers
    • Generic API keys and secrets

Missing Input Validation on Webhook Payloads FIXED (P2-061)

FIXED: 2026-01-29

Changes made:

  • Added MAX_PAYLOAD_SIZE constant (1 MB limit)
  • Added MAX_JSON_DEPTH constant (32 levels)
  • Added validatePayloadSize() method - rejects payloads > 1MB
  • Added parseAndValidateJson() method - validates JSON with depth limit
  • Added validatePayloadStructure() method - provider-specific validation
  • Added hasExcessiveArraySize() method - prevents DoS via large arrays
  • Added comprehensive unit tests in tests/Unit/WebhookPayloadValidationTest.php

P2 - High Priority

Missing Table Name on Vendor Model FIXED

FIXED: 2026-01-29 (as part of P2-058 migration fix)

  • Added explicit protected $table = 'uptelligence_vendors'; to Vendor model
  • All models now have explicit table names with uptelligence_ prefix

DiffAnalyzerService Constructor Pattern Inconsistency

DiffAnalyzerService requires a Vendor in constructor unlike other services (which use dependency injection).

File: Services/DiffAnalyzerService.php:27-33

Acceptance criteria:

  • Refactor to accept Vendor as method parameter instead of constructor
  • Update all callers
  • Register as singleton like other services

Missing Process Shell Injection Protection in AssetTrackerService FIXED

The AssetTrackerService uses Process::run() with string interpolation.

FIXED: Now uses array-based Process invocation with package name validation.

Changes made:

  • Use array syntax for Process commands to prevent shell injection
  • Validate package names against allowlist pattern (Composer and NPM patterns)
  • Added tests for shell injection prevention in tests/Unit/AssetTrackerServiceTest.php

VendorStorageService Uses Undefined File Facade

The extractMetadata and getDirectorySize methods use File:: facade which is not imported.

File: Services/VendorStorageService.php:391,417,562

Acceptance criteria:

  • Add use Illuminate\Support\Facades\File; import
  • Or refactor to use Storage facade consistently

Missing Authorisation Checks

No authorisation checks on admin Livewire components - relies entirely on route middleware.

Files affected:

  • View/Modal/Admin/*.php - All admin modals

Acceptance criteria:

  • Add explicit authorisation checks in Livewire components
  • Use Gates/Policies for fine-grained access control
  • Consider workspace-level permissions for multi-tenant isolation

Missing Vendor Model Workspace Scope

The Vendor model doesn't use BelongsToWorkspace trait, but UptelligenceDigest does.

File: Models/Vendor.php

Acceptance criteria:

  • Determine if vendors should be workspace-scoped or global
  • If workspace-scoped, add BelongsToWorkspace trait
  • Add workspace_id to migration

P3 - Medium Priority

TestCase Incorrect Namespace

The test case uses wrong namespace and doesn't extend Orchestra TestCase.

File: tests/TestCase.php

namespace Tests;  // Should be Core\Mod\Uptelligence\Tests

Acceptance criteria:

  • Fix namespace to Core\Mod\Uptelligence\Tests
  • Extend Orchestra\Testbench\TestCase
  • Configure package service provider loading

Missing Test Coverage

The tests/Feature/ and tests/Unit/ directories contain only .gitkeep files.

Acceptance criteria:

  • Add unit tests for all Services
  • Add unit tests for Model methods and scopes
  • Add feature tests for webhook endpoints
  • Add feature tests for console commands
  • Target: 80% code coverage

Missing Rate Limiter Key Consistency

Different services use different rate limiter key formats.

Files affected:

  • Boot.php - Defines limiters
  • Services/AIAnalyzerService.php:221 - Uses key without user context
  • Services/IssueGeneratorService.php:91 - Uses same global key

Acceptance criteria:

  • Standardise rate limiter key naming
  • Consider per-vendor or per-user rate limits where appropriate

Missing Retry Logic Consistency

Some HTTP calls have retry logic, others don't.

File: Services/IssueGeneratorService.php:393-406 - createWeeklyDigest lacks retry logic unlike createGitHubIssue

Acceptance criteria:

  • Add retry logic to all external HTTP calls
  • Extract common HTTP client configuration to shared method

DiffCache Table Name Hardcoded FIXED

FIXED: 2026-01-29 (as part of P2-058 migration fix)

  • Renamed table to uptelligence_diff_cache for consistency
  • Updated model $table property
  • Created migration with correct table name

Missing Carbon Import in UptelligenceDigest

Uses \Carbon\Carbon with full path instead of import.

File: Models/UptelligenceDigest.php:258

Acceptance criteria:

  • Add use Carbon\Carbon; import
  • Replace \Carbon\Carbon with Carbon

Emoji Usage in Models

Models use emoji characters for icons which may cause encoding issues.

Files affected:

  • Models/Vendor.php:208-215
  • Models/UpstreamTodo.php:215-228
  • Models/DiffCache.php:213-250
  • Models/AnalysisLog.php:156-170

Acceptance criteria:

  • Replace emojis with icon class names (Font Awesome)
  • Or create separate icon mapping service
  • Ensure UTF-8 encoding throughout

P4 - Low Priority

Inconsistent Method Naming

Some methods use British spelling, others American.

Examples:

  • normaliseVersion (British) in WebhookReceiverService.php
  • normaliseVersion (British) in VendorUpdateCheckerService.php
  • Consistent, but verify across codebase

Acceptance criteria:

  • Audit all method names for spelling consistency (prefer British)
  • Document spelling conventions in CLAUDE.md

Missing PHPDoc Return Types

Some methods lack @return documentation.

Files affected: Most service methods

Acceptance criteria:

  • Add comprehensive PHPDoc blocks to all public methods
  • Include @param, @return, @throws annotations

Console Commands Missing Progress Bars

Commands don't show progress for long-running operations.

Files affected:

  • Console/CheckCommand.php
  • Console/AnalyzeCommand.php
  • Console/IssuesCommand.php

Acceptance criteria:

  • Add progress bars for iterating over vendors/assets
  • Add --verbose option for detailed output

Missing Soft Deletes on Some Models

DiffCache and AnalysisLog lack soft deletes while related models have them.

Acceptance criteria:

  • Add SoftDeletes trait to DiffCache and AnalysisLog
  • Add deleted_at column to migrations

Missing Model Events

No model observers or events for audit logging.

Acceptance criteria:

  • Add observer for UpstreamTodo status changes
  • Add events for version detection, analysis completion
  • Integrate with activity logging system

P5 - Nice to Have

Add MCP Tool Integration

The package is designed for AI analysis but has no MCP tool handlers.

Acceptance criteria:

  • Add McpToolsRegistering event listener to Boot.php
  • Create MCP tools for:
    • Listing pending todos
    • Checking vendor status
    • Triggering analysis
    • Getting quick wins summary

Add WebSocket/Broadcast Support

No real-time updates for webhook deliveries or analysis progress.

Acceptance criteria:

  • Add Laravel Echo events for webhook received
  • Broadcast analysis progress updates
  • Add Livewire polling as fallback

Add Slack/Discord Notifications

Config has webhook URLs but no implementation.

File: config.php:223-225

Acceptance criteria:

  • Implement Slack notification channel
  • Implement Discord notification channel
  • Add notification preferences per user

Support GitLab Self-Hosted

isGiteaUrl method only checks configured Gitea host.

File: Services/VendorUpdateCheckerService.php:409-418

Acceptance criteria:

  • Add support for GitLab self-hosted instances
  • Make git host detection more flexible

Add Changelog Parsing

Extract structured changelog data from releases.

Acceptance criteria:

  • Parse Keep a Changelog format
  • Extract breaking changes automatically
  • Link changelog entries to todos

P6+ - Future / Backlog

Database Performance Optimisation

  • Add database indexes for common query patterns
  • Implement query caching for dashboard stats
  • Consider read replicas for analytics queries

Multi-Language Support

  • Extract all user-facing strings to lang files
  • Add translation support for notifications

API Endpoints for External Integration

  • Create REST API for querying todos
  • Add GraphQL support
  • Implement API versioning

Archive Management UI

  • Add S3 storage browser in admin
  • Implement bulk archive/restore operations
  • Add storage quota monitoring

Advanced AI Analysis

  • Implement multi-file context analysis
  • Add code complexity metrics
  • Generate migration scripts automatically

Vendor Dependency Graph

  • Track inter-vendor dependencies
  • Visualise dependency tree
  • Detect cascading update requirements

Completed

2026-01-29 P2 Security & Infrastructure Fixes

  • P2-058: Migration Mismatch - Created vendor tracking migration (0001_01_01_000004_create_uptelligence_vendor_tables.php), added explicit $table properties to all models with uptelligence_ prefix, clarified dual-purpose nature (uptime + vendor tracking)
  • P2-059: Webhook Signature Timing Attack Audit - Verified all signature verification uses hash_equals(), added comprehensive tests in tests/Unit/WebhookSignatureVerificationTest.php
  • P2-060: API Key Exposure in Logs - Added redactSensitiveData() method to AIAnalyzerService, IssueGeneratorService, and VendorUpdateCheckerService to redact API keys, tokens, and credentials from log output
  • P2-061: Missing Webhook Payload Validation - Added payload size limit (1MB), JSON depth limit (32), provider-specific schema validation, array size limits, tests in tests/Unit/WebhookPayloadValidationTest.php
  • P3-DiffCache Table Name - Fixed table name from diff_cache to uptelligence_diff_cache for consistency

2026-01-21 Code Review Wave

Items completed as part of the 2026-01-21 code review wave (per changelog/2026/jan/code-review.md):

  • Path traversal validation - Added to DiffAnalyzerService::validatePath() to prevent directory traversal attacks
  • Shell injection fix in DiffAnalyzerService - Now uses array syntax: Process::run(['diff', '-u', $prevPath, $currPath])
  • Shell injection fix in AssetTrackerService - Now uses array syntax with package name validation for all Process::run() calls
  • Rate limiting for AI API calls - Implemented in AIAnalyzerService with configurable limit
  • Retry logic with exponential backoff - Added to Packagist, NPM, GitHub, Gitea, Anthropic, and OpenAI API calls
  • Enhanced error logging - Improved logging for API failures beyond just report($e)
  • Database transactions - Added to DiffAnalyzerService::cacheDiffs() with rollback on failure
  • Soft deletes - Added to relevant models (Vendor, VersionRelease, UpstreamTodo, UptelligenceWebhook)
  • Database indexes - Added indexes on diff_cache.version_release_id and upstream_todos.vendor_id
  • Target repo validation - Added validation in IssueGeneratorService before using explode('/', ...)
  • Storage facade standardisation - VendorStorageService now uses Storage facade consistently
  • Config validation on boot - Added warnings for missing API keys in Boot::validateConfig()
  • Timestamp validation - Fixed released_at parsing in AssetTrackerService::parseReleaseTimestamp()
  • Agentic module soft dependency - UpstreamPlanGeneratorService now checks module availability
  • Webhook system - Implemented inbound webhook endpoints, signature verification, and async processing