# SPRINT 0 — ARCHITECTURE REVIEW | Field | Value | |-------|-------| | Document Type | Architecture Review | | Subject | Inveetaire Sprint 0 Framework | | Reviewer | Independent AI Architecture Audit | | Review Date | 2026-07-01 | | Framework Status | Sprint 0 Complete | | Repository Commit | e961697 (HEAD, main) | --- ## 1. Executive Summary The Sprint 0 framework is **production-quality infrastructure**. The 15 core kernel files are cohesive, correctly layered, and well-documented. The codebase demonstrates disciplined engineering: no business logic in the kernel, no premature abstractions, no shortcuts on security. The architecture is consistent with the approved documents (DBP v1.1, MIB v1.1, ADR) at every layer. **The framework is ready to support Sprint 1 development.** One **medium** technical debt item (Content Security Policy) and three **low** items are identified. None block sprint advancement. All are expected deferred responsibilities documented in the approved architecture. --- ## 2. Architecture Scorecard | Dimension | Score | Notes | |-----------|-------|-------| | Layered Architecture | ✅ **Excellent** | Dependency direction enforced, no violations found | | Kernel Implementation | ✅ **Excellent** | 15 classes — correct, production-ready | | Security Posture | ✅ **Strong** | Output escaping, CSRF infrastructure, session flags, Apache hardening | | Maintainability | ✅ **Excellent** | Consistent naming, small focused files, no tangled dependencies | | Scalability | ✅ **Appropriate** | Single-process PHP is intentional for hosting constraints | | Documentation Quality | ✅ **Outstanding** | Inline docblocks, cross-referenced to approved docs | | Technical Debt | ⚠️ **Low** | 1 medium + 3 low items; all expected and documented | | Sprint 1 Readiness | ✅ **READY** | No blockers | --- ## 3. Architecture Overview ### 3.1 Request Lifecycle ``` Browser Request │ ▼ .htaccess (root) ── blocks app/, config/, storage/, routes/ │ ▼ public/.htaccess ── routes to public/index.php │ ▼ public/index.php │ defines ROOT_PATH ▼ Application::run() │ ├── Bootstrap::init() │ ├── 1. loadEnvironment() (.env → $_ENV) │ ├── 2. loadHelpers() (env, config, csrf_field, escape_html) │ ├── 3. configureErrorReporting() (APP_DEBUG gate) │ ├── 4. registerErrorHandlers() (ErrorHandler::register()) │ ├── 5. setTimezone() (UTC default) │ └── 6. startSession() (Session::start()) │ └── Application::handleRequest() │ ├── Router::loadRoutes() │ (5 route files) │ ├── Request::fromGlobals() │ └── Router::dispatch() │ ├── Pattern match → route │ ├── MiddlewarePipeline::run() │ (middleware chain, currently empty in Sprint 0) │ └── Controller::method(Request) │ └── BaseController::{render|redirect|json} ``` ### 3.2 Dependency Direction The dependency graph is **strictly top-down**. No kernel class imports a module class. No lower-layer class references a higher-layer class. ``` public/index.php → Application → Bootstrap → ErrorHandler → Logger → Session → Router → Request → MiddlewarePipeline → [Controller extends BaseController] → [Service extends BaseService] → [Model extends BaseModel] → Database (PDO singleton) ``` **Finding**: Dependency inversion is correctly maintained. No circular dependencies detected. --- ## 4. Framework Kernel Review ### 4.1 Bootstrap.php ✅ **Strengths**: - Boot sequence is explicitly numbered and documented - Autoloader registration is intentionally absent (autoloader self-registers via `spl_autoload_register` in Autoloader.php, loaded before Bootstrap through `public/index.php`) - `loadEnvironment()` correctly guards against overwriting server-level env vars (`array_key_exists` check before `$_ENV[$key] = $value`) - `.env` key validation regex (`/^[A-Z][A-Z0-9_]*$/`) prevents injection from malformed `.env` files - Session start correctly deferred to after error handler registration **Findings**: None adverse. One minor note: Bootstrap comment on line 17 says "Step 6 — Set default timezone" but the docblock above `setTimezone()` numbers it Step 5 (sequence comment inconsistency). This is cosmetic only — the actual execution order is correct. ### 4.2 Application.php ✅ **Strengths**: - Clean separation: `run()` → `bootstrap.init()` → `handleRequest()` - Accepts optional Bootstrap injection (testability hook) - Thin: 84 lines, no business logic **Finding**: `Application.php` comments describe INIT task milestones inline (e.g., "Session start is added here in INIT-010"). These are appropriate historical notes for an AI-assisted project. They could be cleaned up before open-sourcing but are not technical debt for a closed team project. ### 4.3 Autoloader.php ✅ **Strengths**: - Pure `spl_autoload_register` — no Composer dependency in the kernel - Namespace `App\` maps to `app/`; handles subdirectory nesting correctly - No Reflection API used ### 4.4 Request.php ✅ **Strengths**: - Subdirectory base-path stripping correctly handles XAMPP `/inveetaire/public/` installations and production root installs - `fromGlobals()` factory pattern is clean - `isPost()`, `get()`, `post()`, `param()`, `header()`, `ip()` API is complete and sufficient for all Sprint 1 needs - POST data is not mutated or filtered at this layer (correct — filtering is a Service responsibility) **Finding**: `Request` is not truly immutable — `setParams()` is public to allow Router injection. This is a documented constraint and correct for this architecture. No issue. ### 4.5 Router.php ✅ **Strengths**: - Regex-based named parameter extraction is correct and safe - Group support (prefix + middleware inheritance) is clean - Route files loaded via `include $file` inside a bounded scope — no accidental variable leakage - 404 delegated to `ErrorHandler::renderNotFound()` — no raw echo - Middleware names stored per-route (not yet executed — deferred to `MiddlewarePipeline`) **Finding**: Route method validation enforces GET/POST only — correct per DBP. No hidden method override (no `_method` field) — correct for MVP. ### 4.6 MiddlewarePipeline.php ✅ **Strengths**: - Onion/decorator pattern implemented correctly (reverse iteration, `array_reverse`) - `class_exists()` triggers PSR-4 autoload — no Reflection - Missing class and missing `handle()` method both produce logged errors, not PHP fatals - Empty stack (Sprint 0 state) calls destination directly with zero overhead **Finding**: The middleware contract is by convention (`handle(Request, callable)`) — no interface file enforces it. This is documented (MIB INIT-009 note: "Middleware contract — by convention"). This is **low technical debt** — a `MiddlewareInterface` should be added before concrete middleware is built in Sprint 1. ### 4.7 Session.php ✅ **Strengths**: - `$started` static flag prevents double `session_start()` calls - Save path fallback logic is graceful (warns + continues with system default) - Cookie params: `httponly=true`, `samesite=Strict`, `secure` driven by `APP_ENV` - `regenerate()` correctly documented as a privilege-escalation step (Auth module concern) - `destroy()` correctly clears cookie and session data in correct order **Finding**: `secure` cookie flag is driven by `APP_ENV === 'production'`. This is correct and intentional for XAMPP development. Ensure production deploy sets `APP_ENV=production` in `.env` — not an implementation gap, a deployment checklist item. ### 4.8 Csrf.php ✅ **Strengths**: - `bin2hex(random_bytes(32))` — cryptographically secure token generation (CSPRNG) - `hash_equals()` used for comparison — timing-safe, prevents oracle attacks - Token generation is idempotent (`getToken()` returns existing token if present) - `invalidate()` method available for logout flows - `validate()` correctly passes non-POST requests unconditionally **Finding**: `CsrfMiddleware` does not yet exist (deferred to Sprint 1 per REV-013). Until it is built, CSRF validation is not wired into the request pipeline. This is **not a bug** — no POST-handling routes exist yet. The infrastructure is correct and ready. ### 4.9 Database.php ✅ **Strengths**: - True singleton via static private `?PDO $connection` - Private constructor prevents instantiation - `PDO::ATTR_EMULATE_PREPARES = false` — real prepared statements enforced - `PDO::ERRMODE_EXCEPTION` — all errors throw, none silent - DSN redacts `dbname` in log output on connection failure (credentials not logged) - `reset()` and `setConnection()` test hooks are documented as test-only **Finding**: None adverse. ### 4.10 BaseModel.php ✅ **Strengths**: - Lazy connection resolution — DB not touched until first query - `$table` abstract property forces concrete models to declare their table - `query()` returns `PDOStatement` — allows `fetchAll()`, `fetch()`, `rowCount()` flexibility - Core primitives (`find`, `findAll`, `insert`, `update`, `count`) are correct and consistent - `count()` returns `int` — matches DBP documentation **One gap found**: The `BaseModel` docblock example (lines 25–30) references `->fetchAll()` on the result of `query()`. However, `findAll()` and `find()` themselves return arrays — correct. The example in the docblock is illustrative of a custom model method calling `query()` directly, which is acceptable. No implementation gap. ### 4.11 BaseController.php ✅ **Strengths**: - Output buffering with `ob_start()` / `ob_get_clean()` is correct - View file existence check before include — 404 logged and rendered - Layout fallback (no layout file → raw view output) is defensive and documented - `json()` sets `Content-Type: application/json` header before `echo` - `redirect()` calls `exit` after header — prevents further output - Flash messages pulled by `pullFlash()` in render — not by the layout directly (correct separation) **Finding**: `$layout = 'app'` default produces unstyled output for any controller that forgets to set a portal layout. This is documented (REV-017, MIB, DBP) as an intentional no-layout fallback. Sprint 1 controllers must set `$layout` explicitly — this is an enforcement gap but not a framework bug. ### 4.12 BaseService.php ✅ **Strengths**: - `config()` and `env()` wrapper methods provide consistent service-layer access to config - Abstract class forces subclassing - No database access at this level — models are injected/instantiated by concrete services - Clear documentation of what services must NOT do (no browser output, no headers) ### 4.13 Logger.php ✅ **Strengths**: - Never throws — falls back to `error_log()` on write failure - No database dependency — can log before DB is available - Log path resolved with `ROOT_PATH` fallback - `file_put_contents()` with `FILE_APPEND | LOCK_EX` — atomic, thread-safe writes - UTC timestamps enforced via `gmdate()` - User context placeholder (`-`) for pre-auth requests — correct for Sprint 0 - Log format: `[timestamp] [LEVEL] [METHOD path] [user] Message {context_json}` — machine-parseable **Finding**: Log files are not rotated — they grow indefinitely per day. This is appropriate for shared hosting (no log rotation daemon). **Low technical debt** — a size-based rotation or cleanup script should be considered before production launch, not before Sprint 1. ### 4.14 ErrorHandler.php ✅ **Strengths**: - All three PHP error channels covered: `set_error_handler`, `set_exception_handler`, `register_shutdown_function` - `@` operator suppression is correctly honoured (`error_reporting() & $errno` check) - Debug page shows full stack trace with syntax-highlighted code context - Production page renders `errors/500.php` — no raw PHP output exposed - 404, 403, 503 (maintenance) helpers available for Router use - HTTP status codes set before view include — correct ### 4.15 View Renderer (BaseController::render + Layouts) ✅ **Strengths**: - Two-pass buffered render: view into buffer → inject as `$content` → layout wraps - All 4 portal layouts present (couple, admin, crew, public) - All layouts use `escape_html()` on variable output - Flash messages rendered via component — not inline - TailwindCSS and Alpine.js loaded via CDN — correct Sprint 0 approach - `noindex, nofollow` meta tag on portal layouts — correct (no public SEO) **Finding**: TailwindCSS is loaded via CDN script tag with inline config. In production, this must be replaced with a compiled CSS build. This is a documented Sprint 0 decision (not technical debt — it is the approved approach for this stage). --- ## 5. Security Review ### 5.1 Output Escaping ✅ `escape_html()` enforces `htmlspecialchars($value, ENT_QUOTES, 'UTF-8')`. All layouts use it for variable output. The rule is documented in DBP, MIB, and in the helper's own docblock. The framework makes the correct pattern easy and the wrong pattern visible. **Gap**: Enforcement is by convention, not by the type system. A developer writing a new view could `echo $var` directly. This is inherent to PHP template files and cannot be structurally prevented. The documentation-level rule is the correct mitigation. ### 5.2 Session Security ✅ | Flag | Status | |------|--------| | `HttpOnly` | ✅ `true` — JS cannot steal cookie | | `SameSite` | ✅ `Strict` — CSRF mitigation at cookie level | | `Secure` | ✅ `true` in production (env-driven) | | Custom save path | ✅ `storage/sessions/` — not system temp | | Session ID regeneration on login | ⏳ Deferred to Auth (Sprint 2) — documented | | Absolute + inactivity timeouts | ⏳ Deferred to Auth (Sprint 2) — infrastructure ready | ### 5.3 CSRF ✅ | Item | Status | |------|--------| | Token generation | ✅ `bin2hex(random_bytes(32))` — CSPRNG | | Token comparison | ✅ `hash_equals()` — timing-safe | | Token storage | ✅ `$_SESSION['csrf_token']` | | Token delivery | ✅ `csrf_field()` helper | | Middleware enforcement | ⏳ `CsrfMiddleware` deferred to Sprint 1 — no POST routes yet | | AJAX support | ⚠️ Not yet addressed — AJAX CSRF pattern needs to be defined before AJAX endpoints are built | ### 5.4 Error Handling ✅ - No stack traces exposed in production (`APP_DEBUG=false`) - DB credentials not logged on connection failure (DSN redacted) - Logger never throws — error reporting failure cannot produce a secondary error ### 5.5 Logging ✅ - User/IP context captured per request - Sensitive values not written to log (no password, no token values) - Log files stored outside web root (`storage/logs/`) ### 5.6 Configuration & Environment Loading ✅ - `.env` key format validated by regex before insertion into `$_ENV` - Server-level env vars not overwritten — CI/CD compatible - `.env` excluded from git (`.gitignore`) - Config files return arrays — not executed user input ### 5.7 Apache Rules ✅ | Protection | Status | |------------|--------| | Directory listing disabled | ✅ `Options -Indexes` | | `app/`, `config/`, `storage/`, `routes/` blocked | ✅ `[F,L]` rewrite + `Require all denied` | | Dotfile access blocked | ✅ `` in layouts. **Blocks Sprint 1?**: No — no user content is rendered yet. --- ### TD-003 — Log File Rotation Absent **Classification**: Low **Location**: `app/core/Logger.php` **Description**: Log files grow indefinitely within each calendar day. There is no size-based rotation or archive mechanism. **Impact**: Low — on shared hosting with moderate traffic, daily files will not reach problematic sizes in early sprints. This becomes relevant post-launch. **Resolution**: A cleanup script or `logrotate` config before production launch. Not a code change. **Blocks Sprint 1?**: No. --- ### TD-004 — TailwindCSS CDN in Production Layouts **Classification**: Low **Location**: `app/views/layouts/*.php` **Description**: All four layouts load TailwindCSS via CDN `