PR-1: Migrations & Base Schema (tenant_keys, person) #52
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Install laravel/sanctum ^4.2 and spatie/laravel-permission ^6.22 - Create tenant_keys table with envelope key storage (DEK/idx_key as BYTEA) - Create person table with encrypted fields (BYTEA), blind indexes, and tsvector for FTS - Add PostgreSQL GIN index on note_tsv for full-text search - Add composite indexes on (tenant_id, email_idx) and (tenant_id, phone_idx) - Switch phpunit.xml to use pgsql for testing (remove sqlite in-memory DB) - Add comprehensive PEST schema tests for both tables - Verify BYTEA types, tsvector type, indexes, and foreign key constraints Relates to #50 (PR-1)
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds encryption key management and encrypted person data storage with PostgreSQL-specific features. The changes transition from SQLite (in-memory) to PostgreSQL for testing and introduce two new database tables with cryptographic columns.
Key changes:
- Added
tenant_keysandpersontables with binary encryption fields - Switched test database from SQLite to PostgreSQL
- Added new dependencies:
laravel/sanctumandspatie/laravel-permission
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
database/migrations/2025_11_01_165633_create_tenant_keys_table.php |
Creates table for storing wrapped encryption keys with binary columns |
database/migrations/2025_11_01_165901_create_person_table.php |
Creates person table with encrypted fields and PostgreSQL tsvector for search |
tests/Feature/TenantKeysSchemaTest.php |
Tests schema structure and column types for tenant_keys table |
tests/Feature/PersonSchemaTest.php |
Tests schema structure, indexes, and foreign keys for person table |
phpunit.xml |
Changes test database from SQLite in-memory to PostgreSQL |
composer.json |
Adds Laravel Sanctum and Spatie Permission packages |
composer.lock |
Lock file updates for new dependencies |
- Replace reusable workflow call with inline job + PostgreSQL service - PostgreSQL 17-alpine service with health checks - Configure DB env vars for test job (localhost:5432) - Add driver check for PostgreSQL-specific tsvector/GIN index - All 17 tests pass (no skipped tests) - Pint, PHPStan, PEST all pass Addresses Copilot review comment about database-specific SQL Fixes CI test failures (was using SQLite, now PostgreSQL)
- Add explicit return types to all test closures per Laravel guidelines - Add test for note_enc column in PersonSchemaTest (TEXT type verification) - Add DB_DATABASE=testing to phpunit.xml to avoid .env fallback confusion - Replace reusable quality workflow with inline PostgreSQL service job (Quality Checks PEST tests were failing with SQLite, now use postgres:17-alpine) - Both PHP CI and Quality Checks workflows now use identical PostgreSQL setup This resolves all 5 Copilot review comments: 1. Driver check for tsvector (already fixed in previous commit) 2. Missing return types in test closures 3. Missing note_enc column test 4. DB_DATABASE env var missing 5. Quality workflow tests failing due to missing PostgreSQL service All tests pass locally (18 passed, 42 assertions) with DDEV PostgreSQL.
Required status check expects 'PEST Tests / Run Tests' but step was named 'Run PEST tests'. Rename to 'Run Tests' to match branch protection rule.
This was referenced Nov 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements migrations for envelope key management (
tenant_keys) and encrypted contact data (person) with blind indexes and full-text search support.Changes
laravel/sanctum ^4.2andspatie/laravel-permission ^6.22dek_wrapped,dek_nonce,idx_wrapped,idx_nonce,key_version(INT, default 1)email_enc,phone_enc,note_enc(BYTEA/TEXT)email_idx,phone_idx(BYTEA) for equality search without decryptionnote_tsv(tsvector) with GIN index(tenant_id, email_idx)and(tenant_id, phone_idx)tenant_id→tenant_keys.idwith cascade deletephpunit.xmlfrom SQLite to PostgreSQL (aligns with production)Testing
@phpstan-ignore property.nonObjectfor DB result objects)Size
Relates to
Notes
DB::statement()for PostgreSQL-specific types (tsvector, GIN index)$column->propertyaccess on DB result objects (expected mixed type)db_testdatabase created manually for test environment (CI uses service container)