From cdb7dddd440aa915a0aa6eaad2b0fc1d33a73255 Mon Sep 17 00:00:00 2001 From: Clotho Date: Fri, 20 Feb 2026 16:08:46 +0000 Subject: [PATCH] docs(phase-0): environment assessment, architecture review, and findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves #2 (phase 0 tasks): - FINDINGS.md: full environment + architecture report - composer install blocked by missing host-uk/core repository config - test baseline: 15 files / ~4,053 lines (cannot run, vendor missing) - code quality: 9 files missing declare(strict_types=1) - architecture documented: BelongsToWorkspace, WorkspaceScope, middleware chain, migration patterns, entitlement system - critical bug identified: namespace_id columns missing from entitlement_usage_records and entitlement_boosts migrations - TODO.md: three new items added - BUG-001 (P1): missing namespace_id migration columns — runtime crash - DX-005 (P3): strict_types in 9 remaining model/service files - ENV-001 (P3): configure host-uk/core repository so composer install works Co-Authored-By: Clotho --- FINDINGS.md | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++ TODO.md | 42 +++++++++ 2 files changed, 290 insertions(+) create mode 100644 FINDINGS.md diff --git a/FINDINGS.md b/FINDINGS.md new file mode 100644 index 0000000..23e6d64 --- /dev/null +++ b/FINDINGS.md @@ -0,0 +1,248 @@ +# Phase 0 Findings — core/php-tenant + +**Date:** 2026-02-20 +**Branch:** feat/phase-0-assessment +**Analyst:** Clotho (darbs-claude) +**Issue:** #2 + +--- + +## 1. Environment + +| Tool | Version | Status | +|----------|-------------|---------| +| PHP | 8.3.6 | OK | +| Composer | 2.9.5 | OK | +| Vendor | — | MISSING | + +### 1.1 Composer Install + +``` +composer install --no-interaction +``` + +**Result: FAILED** + +``` +Your requirements could not be resolved to an installable set of packages. + + Problem 1 + - Root composer.json requires host-uk/core, it could not be found in any version, + there may be a typo in the package name. +``` + +**Root cause:** `host-uk/core` (the host framework) is a private package. The `composer.json` has no `repositories` section to point Composer at the private registry or a local path. + +**Resolution needed:** One of: +1. Add a `repositories` entry pointing to the Forgejo package registry (`darbs.lthn.ai`), or +2. Set `COMPOSER_AUTH` env var with appropriate credentials, or +3. Add a local path repository for development if `core` is checked out alongside this package. + +Until resolved, **all tooling that requires `vendor/` is blocked** (tests, Pint, PHPStan). + +--- + +## 2. Test Baseline + +**Status: CANNOT RUN** — vendor directory is empty. + +### Test inventory (static count) + +| File | Lines | +|------|-------| +| `tests/Feature/AccountDeletionTest.php` | ~120 | +| `tests/Feature/AuthenticationTest.php` | ~160 | +| `tests/Feature/EntitlementApiTest.php` | ~700 | +| `tests/Feature/EntitlementServiceTest.php` | ~800 | +| `tests/Feature/ProfileTest.php` | ~100 | +| `tests/Feature/ResetBillingCyclesTest.php` | ~180 | +| `tests/Feature/SettingsTest.php` | ~120 | +| `tests/Feature/TwoFactorAuthenticatableTest.php` | ~140 | +| `tests/Feature/UsageAlertServiceTest.php` | ~180 | +| `tests/Feature/WaitlistTest.php` | ~120 | +| `tests/Feature/WorkspaceCacheTest.php` | ~300 | +| `tests/Feature/WorkspaceInvitationTest.php` | ~255 | +| `tests/Feature/WorkspaceSecurityTest.php` | ~433 | +| `tests/Feature/WorkspaceTenancyTest.php` | ~165 | +| `tests/Feature/Guards/AccessTokenGuardTest.php` | ~180 | +| **Total** | **~4,053 lines across 15 files** | + +Tests use Pest with Orchestra Testbench. Coverage appears comprehensive for workspace- and entitlement-level tests. + +**Pass/fail counts:** Cannot determine — blocked by missing vendor. + +--- + +## 3. Code Quality (Static Review) + +### 3.1 Pint / PHPStan + +Cannot run — vendor missing. Tools (`vendor/bin/pint`, `vendor/bin/phpstan`) not available. + +### 3.2 Missing `declare(strict_types=1)` — identified via static scan + +The following files are missing the strict types declaration despite this being a documented coding standard: + +| File | Priority | +|------|----------| +| `Models/AccountDeletionRequest.php` | P3 | +| `Models/Boost.php` | P3 | +| `Models/EntitlementLog.php` | P3 | +| `Models/Feature.php` | P3 | +| `Models/Package.php` | P3 | +| `Models/UsageRecord.php` | P3 | +| `Models/WaitlistEntry.php` | P3 | +| `Models/WorkspacePackage.php` | P3 | +| `Services/EntitlementResult.php` | P3 | + +> NOTE: TODO DX-001 records this as fixed for `Models/Workspace.php`, `Models/User.php`, and `Services/EntitlementService.php` — but 9 other files still lack the declaration. + +--- + +## 4. Architecture Review + +### 4.1 Workspace Isolation — BelongsToWorkspace Trait + +**File:** `Concerns/BelongsToWorkspace.php` + +The trait is the primary tenancy enforcement mechanism for Eloquent models. + +**Isolation mechanisms:** +1. `bootBelongsToWorkspace()` registers an Eloquent `creating` hook that: + - Reads the current workspace from request attributes (`workspace_model`) or the authenticated user's default workspace + - Auto-assigns `workspace_id` if not already set + - Throws `MissingWorkspaceContextException` if workspace context is absent and strict mode is on + +2. `scopeOwnedByCurrentWorkspace(Builder $query)` filters queries to the current workspace. In non-strict mode it returns `whereRaw('1 = 0')` (empty, fail-safe). + +3. Cache invalidation is wired to `saved` and `deleted` model events via `clearWorkspaceCache()`. + +**Opt-out:** Setting `protected bool $workspaceContextRequired = false;` on a model disables the exception — useful for legacy code but discouraged. + +**Workspace resolution order:** +``` +request()->attributes->get('workspace_model') // set by ResolveWorkspaceFromSubdomain + → auth()->user()->defaultHostWorkspace() // falls back to auth user + → null → MissingWorkspaceContextException +``` + +### 4.2 WorkspaceScope — Global Query Scope + +**File:** `Scopes/WorkspaceScope.php` + +An Eloquent global scope (implements `Scope`) that automatically filters all queries by the current workspace. + +Key design decisions: +- `static bool $strictModeEnabled = true` — global toggle +- `withoutStrictMode(callable $callback)` — safe scoped disable (restores on exit) +- Builder macros: `forWorkspace(Workspace|int)`, `acrossWorkspaces()`, `currentWorkspaceId()` +- Console commands bypass strict mode automatically (`runningInConsole()` + not unit tests) +- Models can opt out: `public bool $workspaceScopeStrict = false;` + +### 4.3 Middleware for Tenant Resolution + +**Files:** `Middleware/` + +| Middleware | Purpose | +|-----------|---------| +| `ResolveWorkspaceFromSubdomain` | Resolves workspace from `{slug}.host.uk.com` subdomain. Hardcoded mappings (hub→main, bio→bio, etc.). Sets `workspace_model` on request attributes. | +| `RequireWorkspaceContext` | Validates workspace exists; validates user has access. Resolution order: subdomain attr → `Workspace::current()` → `workspace_id` input → `X-Workspace-ID` header → `?workspace` query param. Logs denied attempts. | +| `CheckWorkspacePermission` | Per-permission authorisation within a resolved workspace. | +| `RequireAdminDomain` | Restricts routes to admin subdomains (hub, www, hestia). | +| `ResolveNamespace` | Resolves current namespace from `?namespace` query, `X-Namespace` header, or session. | + +**Subdomain hardcoding concern:** `ResolveWorkspaceFromSubdomain` contains a hardcoded mapping of subdomains → workspace slugs. Adding a new service workspace requires a code change. Consider moving to database/config. + +### 4.4 Migration Patterns + +**Files:** `Migrations/` + +| Migration | Tables created/modified | +|-----------|------------------------| +| `0001_01_01_000000_create_tenant_tables.php` | 15 tables: users, workspaces, namespaces, entitlement_features, entitlement_packages, entitlement_package_features, entitlement_workspace_packages, entitlement_namespace_packages, entitlement_boosts, entitlement_usage_records, entitlement_logs, user_two_factor_auth, sessions, password_reset_tokens, user_workspace | +| `2026_01_26_000000_create_workspace_invitations_table.php` | workspace_invitations | +| `2026_01_26_120000_create_usage_alert_history_table.php` | usage_alert_history | +| `2026_01_26_140000_create_entitlement_webhooks_tables.php` | entitlement_webhooks, entitlement_webhook_deliveries | +| `2026_01_26_140000_create_workspace_teams_table.php` | workspace_teams | +| `2026_01_29_000000_add_performance_indexes.php` | Indexes on users.tier, namespaces.slug, workspaces.{is_active,type,domain}, user_workspace.team_id, entitlement_usage_records.user_id, entitlement_logs.user_id | + +All tables use appropriate composite indexes on commonly filtered columns. + +--- + +## 5. Critical Bug: Missing `namespace_id` Columns in Entitlement Tables + +**Severity: P1 — Runtime crash** + +### 5.1 Description + +The namespace-level entitlement features (`recordNamespaceUsage`, `provisionNamespaceBoost`) will fail at runtime with a database error because the underlying tables are missing the `namespace_id` column. + +**Models that declare `namespace_id` as fillable:** + +| Model | File | +|-------|------| +| `UsageRecord` | `Models/UsageRecord.php:18` | +| `Boost` | `Models/Boost.php:17` | + +**Service methods that write `namespace_id`:** + +| Method | File | +|--------|------| +| `EntitlementService::recordNamespaceUsage()` | `Services/EntitlementService.php:453` | +| `EntitlementService::provisionNamespaceBoost()` | `Services/EntitlementService.php:1861` | + +**Migrations that DO NOT create the column:** + +- `entitlement_usage_records` — `0001_01_01_000000_create_tenant_tables.php:248-261` +- `entitlement_boosts` — `0001_01_01_000000_create_tenant_tables.php:226-245` + +**Query methods that filter by `namespace_id` without it existing:** + +- `EntitlementService::getNamespaceCurrentUsage()` lines 1605, 1615, 1622 — `WHERE namespace_id = ?` +- `Namespace_::boosts()` relationship — references a non-existent FK + +### 5.2 Impact + +Any call to: +- `$entitlementService->recordNamespaceUsage($namespace, 'links')` +- `$entitlementService->provisionNamespaceBoost($namespace, 'links', [...])` +- `$entitlementService->canForNamespace($namespace, 'links')` (usage query path) + +will throw: +``` +SQLSTATE[HY000]: General error: 1 table entitlement_usage_records has no column named namespace_id +``` + +The test suite in `tests/Feature/EntitlementServiceTest.php` likely exercises these paths and would be failing if runnable. + +### 5.3 Fix Required + +A migration is needed to add `namespace_id` (nullable FK → namespaces, null on delete) to: +1. `entitlement_usage_records` +2. `entitlement_boosts` + +Plus indexes: `(namespace_id, feature_code, recorded_at)` on usage records, `(namespace_id, feature_code, status)` on boosts. + +--- + +## 6. Summary of Open Issues + +| ID | Severity | Description | Status | +|----|----------|-------------|--------| +| BUG-001 | **P1** | `namespace_id` column missing from `entitlement_usage_records` and `entitlement_boosts` migrations — runtime crash | **NEW** | +| DX-003 | P3 | 9 model/service files missing `declare(strict_types=1)` | NEW | +| ENV-001 | P0-blocker | `composer install` fails — `host-uk/core` repository not configured | NEW | +| ARCH-001 | P4 | `ResolveWorkspaceFromSubdomain` hardcodes subdomain→workspace mappings; new services require code change | NEW | + +Existing items from `TODO.md` P1–P3 (SEC-001 through SEC-006, DX-001, DX-002, TEST-001, TEST-002, PERF-001, PERF-002) are marked Fixed in the TODO and have corresponding code in place. These will be verifiable once `composer install` succeeds and tests run. + +--- + +## 7. Recommended Next Steps + +1. **Immediate:** Fix `composer.json` to resolve `host-uk/core` (add repository entry or document setup instructions in README). +2. **Immediate:** Add migration for `namespace_id` columns on `entitlement_usage_records` and `entitlement_boosts` — this is a production crash bug. +3. **Short-term:** Once vendor installs, run full test suite and document pass/fail baseline. +4. **Short-term:** Add `declare(strict_types=1)` to the 9 remaining files. +5. **Medium-term:** Consider moving subdomain→workspace mapping to config/database. diff --git a/TODO.md b/TODO.md index 5c6b7a9..c286091 100644 --- a/TODO.md +++ b/TODO.md @@ -15,6 +15,22 @@ Comprehensive task list for improving the multi-tenancy package. Items are prior ## P1 - Critical / Security +### BUG-001: Missing `namespace_id` columns — namespace entitlements crash at runtime +**Status:** Open (discovered 2026-02-20, issue #2 phase-0 assessment) +**Files:** `Migrations/0001_01_01_000000_create_tenant_tables.php`, `Models/UsageRecord.php`, `Models/Boost.php`, `Services/EntitlementService.php` + +The `entitlement_usage_records` and `entitlement_boosts` tables are missing a `namespace_id` column. +The `UsageRecord` and `Boost` models declare `namespace_id` as `$fillable`, and `EntitlementService` +writes `namespace_id` in `recordNamespaceUsage()` and `provisionNamespaceBoost()`. Any call to either +method will throw a database error at runtime. + +**Acceptance Criteria:** +- Create migration adding `namespace_id` (nullable FK → namespaces, nullOnDelete) to both tables +- Add indexes: `(namespace_id, feature_code, recorded_at)` on usage records; `(namespace_id, feature_code, status)` on boosts +- Verify namespace-level entitlement tests pass after migration + +--- + ### SEC-001: Add rate limiting to EntitlementApiController **Status:** Fixed (2026-01-29) **File:** `Controllers/EntitlementApiController.php` @@ -296,6 +312,32 @@ The workspace creation uses hardcoded domain `'hub.host.uk.com'`. This should be ## P3 - Medium Priority +### DX-005: Add `declare(strict_types=1)` to remaining model/service files +**Status:** Open (discovered 2026-02-20, issue #2 phase-0 assessment) +**Files:** `Models/AccountDeletionRequest.php`, `Models/Boost.php`, `Models/EntitlementLog.php`, `Models/Feature.php`, `Models/Package.php`, `Models/UsageRecord.php`, `Models/WaitlistEntry.php`, `Models/WorkspacePackage.php`, `Services/EntitlementResult.php` + +Nine files are still missing `declare(strict_types=1)` despite it being a documented coding standard (DX-001 only fixed three files). + +**Acceptance Criteria:** +- Add `declare(strict_types=1);` immediately after `