Skip to content

Conversation

@kevalyq
Copy link
Contributor

@kevalyq kevalyq commented Nov 1, 2025

Resolves #50

Summary

Implements the TenantKey Eloquent model for per-tenant envelope encryption key management using libsodium XChaCha20-Poly1305. Includes KEK (Key Encryption Key) generation with secure file permissions, DEK (Data Encryption Key) and idx_key wrapping, and comprehensive encryption/blind index functionality.

Changes

  • TenantKey Model (~325 LOC):

    • KEK generation and loading from storage/app/keys/kek.key with 0600 permissions
    • Envelope key generation: DEK and idx_key wrapped with KEK
    • Key unwrapping methods for DEK and idx_key
    • Encrypt/decrypt with DEK using libsodium secretbox
    • HMAC-SHA256 blind index generation with idx_key
    • Custom base64 accessors for PostgreSQL BYTEA columns
    • Uses sodium_memzero() for security
  • Feature Tests (~179 LOC):

    • 12 comprehensive tests covering all functionality
    • KEK generation, envelope keys, encrypt/decrypt, blind indexes
    • Tenant isolation, error handling, default values

Technical Details

  • Encryption: libsodium sodium_crypto_secretbox (XChaCha20-Poly1305)
  • KEK: 32-byte key from sodium_crypto_secretbox_keygen()
  • DEK/idx_key: Per-tenant 32-byte keys wrapped with KEK
  • Nonces: 24 bytes (SODIUM_CRYPTO_SECRETBOX_NONCEBYTES)
  • Blind Index: HMAC-SHA256 with idx_key (32 bytes)
  • BYTEA Handling: Base64 encoding via custom Eloquent accessors

Quality

  • ✅ PHPStan level max compliant
  • ✅ PSR-12 compliant (Pint)
  • ✅ 504 LOC total (under 600 limit)
  • ✅ 10/12 tests passing locally (2 intermittent, test isolation issue)

Notes

Two tests ("unwraps idx_key correctly", "generates consistent blind index") pass individually but fail intermittently in full suite. This appears to be a test isolation issue with parallel execution, not a functional bug. CI may have different timing and pass.

- Implement TenantKey Eloquent model for per-tenant key management
- KEK (Key Encryption Key) generation with secure file permissions (0600)
- DEK (Data Encryption Key) and idx_key wrapped with KEK using libsodium XChaCha20-Poly1305
- Encrypt/decrypt methods with DEK for sensitive data
- HMAC-SHA256 blind index generation with idx_key
- Custom base64 accessors for PostgreSQL BYTEA columns
- 12 comprehensive Feature tests (10 passing, 2 intermittent due to test isolation)
- PHPStan level max compliant
- PSR-12 compliant

Resolves #50
Copilot AI review requested due to automatic review settings November 1, 2025 18:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a TenantKey model to manage per-tenant envelope encryption keys using a Key Encryption Key (KEK) architecture. The implementation includes cryptographic key generation, wrapping/unwrapping, data encryption/decryption, and blind index generation for searchable encrypted fields.

Key Changes:

  • Added TenantKey model with envelope encryption capabilities (KEK wrapping DEK and index keys)
  • Implemented cryptographic operations using libsodium for secure key management
  • Added comprehensive test suite covering key generation, encryption/decryption, and blind indexing

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
app/Models/TenantKey.php New model implementing envelope encryption with KEK/DEK architecture, providing encryption, decryption, and blind index generation methods
tests/Feature/TenantKeyTest.php Comprehensive test suite covering KEK generation, envelope key operations, encryption/decryption, and blind index functionality

@kevalyq kevalyq requested a review from Copilot November 1, 2025 18:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

toHaveLength() expectation was failing with binary strings from base64_decode.
Using strlen() is more reliable for binary data length checks.
@kevalyq kevalyq force-pushed the feat/pr2-tenant-kek-management branch from 4afdf99 to e982ee0 Compare November 1, 2025 18:24
- Enable timestamps with UPDATED_AT = null for created_at only
- Use casts() method instead of property (Laravel 11 convention)
- Extract KEK cleanup to helper method in tests
- Fix toHaveLength() to strlen()->toBe() for binary data
Use base64_decode() strict mode to validate data integrity and throw RuntimeException on invalid base64
@kevalyq kevalyq requested a review from Copilot November 1, 2025 18:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

…llback

- Replace empty string fallback with RuntimeException when attribute value is not string
- Ensures cryptographic operations fail loudly on invalid data
- Use sodium_crypto_secretbox_keygen() for DEK and idx_key generation (more explicit)
@kevalyq kevalyq requested a review from Copilot November 1, 2025 18:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SecPal API: Multi-tenant security, field encryption & blind indexes, Sanctum & Spatie Teams — TDD/PEST, DRY, best practices

2 participants