Refactor/simplify v4 - #8
Merged
Merged
Conversation
This release introduces three major production-ready features: 1. Online Key Rotation with Progress Tracking - Background batch processing with configurable batch sizes - Job management: start, pause, resume, cancel operations - Real-time progress monitoring via cipher_rotation_progress() 2. Encryption Statistics & Metrics View - cipher_encryption_stats(): Per-table encryption statistics - cipher_key_usage_stats(): Key version usage breakdown - cipher_metrics(): JSON format for monitoring systems 3. Encryption Coverage Audit - cipher_coverage_audit(): Detailed column-by-column audit - cipher_coverage_summary(): High-level encryption status New files: - column_encrypt--3.0--3.1.sql: Upgrade path from 3.0 - column_encrypt--3.1.sql: Fresh install script Also includes comprehensive test coverage and documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add 3.0 to 3.1 upgrade path verification (PG 14 and 18) - Add tests for new production operations features - Verify cipher_rotation_jobs table creation - Verify new functions: cipher_encryption_stats, cipher_coverage_audit, cipher_start_rotation_job, is_key_loaded, cipher_metrics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cipher_metrics() returns a TABLE, not a scalar JSON value. Changed test to verify it returns rows instead of casting to jsonb. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This major refactoring introduces a simplified encrypt.* schema API and removes bloat accumulated in previous versions. v3.3 (Deprecation Release): - Introduces encrypt schema with clean API functions - Automatic log masking (no cipher_key_disable_log ceremony) - Single column_encrypt_user role (replaces 3-role system) - Old functions remain but emit deprecation notices - Provides migration path to v4.0 v4.0 (Clean Release): - Only 10 functions in encrypt schema: register_key, load_key, unload_key, activate_key, revoke_key, rotate, verify, keys, status, blind_index - Removes deprecated: rate limiting, coverage audit, rotation jobs, audit logging, monitoring/metrics functions - Fresh installs get clean minimal footprint Upgrade Path: 3.1 -> 3.3 -> 4.0 Tests updated for v4.0 API using single unified role. CI workflow updated with v3.3 and v4.0 upgrade path tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- CLAUDE.md: Developer guide for working with the codebase - MIGRATION.md: User guide for upgrading from v3.x to v4.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add REVOKE FROM PUBLIC for encrypt schema functions (security fix) - Grant loaded_cipher_key_versions() to column_encrypt_user - Remove old blind_index function calls from tests (not in v4.0) - Fix DEK length validation test (minimum 16 bytes) - Update expected output to match PostgreSQL 17 format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace old function references with encrypt.* schema API - Update security model to single column_encrypt_user role - Remove Production Operations section (deprecated in v4.0) - Add v4.0 upgrade notes with MIGRATION.md reference - Update key rotation and blind indexing examples Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing REVOKE EXECUTE ON FUNCTION encrypt.blind_index(TEXT, TEXT) FROM PUBLIC to maintain consistent access control across all encrypt.* functions. Previously, this function was granted to column_encrypt_user but not revoked from PUBLIC, unlike all other encrypt.* functions. Files updated: - column_encrypt--4.0.sql (fresh install) - column_encrypt--3.3.sql (deprecation release) - column_encrypt--3.1--3.3.sql (upgrade script) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add CI badge to show build status - Add PostgreSQL version badge (14-18) - Fix Requirements to reflect tested versions (14-18, not 10+) - Fix encrypt.rotate default: batch_size DEFAULT 10000 (was NULL) - Fix encrypt.verify default: sample_size DEFAULT 100 (was 1000) - Update Docker testing section with all supported versions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix CI failure caused by 'test-key' (8 bytes) being too short. The encrypt.register_key() function requires minimum 16 bytes and recommends 32 bytes for AES-256. Changed test key from 'test-key' to 'my-32-byte-test-encryption-key!!' which is exactly 32 bytes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Keys are session-scoped, so load_key must be in the same psql session as INSERT/SELECT operations that need the key. Previous CI ran these in separate sessions causing "key not loaded" error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add .github/copilot-instructions.md - the official location GitHub Copilot reads for repository context during code review. Includes: - v4.0 API reference - Security model overview - Code review checklist - Common patterns and anti-patterns - File structure guide Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a detailed prompt template for requesting full maintainer-grade repository assessments covering architecture, security, API complexity, tests, and simplification opportunities. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers the v4.0 “clean” release of column_encrypt, centered on a simplified encrypt.* schema API with a single column_encrypt_user role, while providing an upgrade path via a v3.3 deprecation release and CI validation for supported upgrade paths.
Changes:
- Adds v4.0 clean install SQL and v3.3→v4.0 upgrade script that removes deprecated functions/features.
- Updates regression tests + expected output to exercise the new
encrypt.*API and role model. - Updates docs and CI workflow to reflect/validate the v3.1→v3.3→v4.0 migration path and supported PostgreSQL versions.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
sql/column_encrypt.sql |
Refactors regression tests to use the v4.0 encrypt.* API and single-role model. |
expected/column_encrypt.out |
Updates expected output to match new test flow and v4.0 API behavior. |
column_encrypt.control |
Sets default extension version to 4.0 and updates extension description. |
column_encrypt--4.0.sql |
Introduces the v4.0 clean install script defining the encrypt schema API + unified role grants. |
column_encrypt--3.3--4.0.sql |
Upgrade script dropping deprecated objects for the v4.0 clean release. |
column_encrypt--3.1.sql |
Adds v3.1 install script (production ops features) to support tested upgrade paths. |
column_encrypt--3.1--3.3.sql |
Adds v3.1→v3.3 upgrade script introducing encrypt.* API and deprecation notices. |
column_encrypt--3.0--3.1.sql |
Adds v3.0→v3.1 upgrade script (ops features) for upgrade-path completeness. |
Makefile |
Ships additional versioned SQL/upgrade scripts with the extension build. |
README.md |
Updates docs for v4.0 API, role model, and upgrade path guidance. |
MIGRATION.md |
Adds a dedicated migration guide for v3.x→v4.0. |
.github/workflows/ci.yml |
Extends CI to validate v3.1→v3.3 and v3.3→v4.0 upgrades and v4.0 fresh installs. |
CLAUDE.md |
Adds repository guidance for Claude Code usage and project structure. |
.github/copilot-instructions.md |
Adds repo-specific Copilot guidance (security checklist, API summary, conventions). |
1. CI fresh-install workflow improvements: - Use psql -At for stable output checking with grep -Fx - Run entire workflow as test_user (non-superuser) via -U flag - Proves column_encrypt_user role works end-to-end - Grant CREATE ON SCHEMA public to test_user for table creation 2. Upgrade script now refreshes function bodies: - Add CREATE OR REPLACE for all encrypt.* functions in 3.3→4.0 - Ensures upgraded databases have identical function bodies to fresh installs - Fixes: v3.3 had different formatting, error messages, and logic 3. Documentation updated for v4.0 error messages: - README: Changed example from EDB-ENC0012 to "incorrect passphrase" - README: Rewrote Error Codes section with v4.0 SQLSTATE codes - MIGRATION.md: Updated troubleshooting section with v4.0 errors - Added note about legacy EDB-ENC* codes from C layer Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add new CI step that: 1. Creates two databases in the same cluster 2. Installs v3.3 and upgrades to v4.0 in one (db_upgraded) 3. Fresh installs v4.0 in the other (db_fresh) 4. Compares MD5 hash of prosrc for all 10 encrypt.* functions 5. Fails with detailed diff if any function body differs This catches regressions where the upgrade script fails to refresh function bodies, ensuring upgraded databases are identical to fresh installs. Functions verified: - encrypt.register_key - encrypt.load_key - encrypt.unload_key - encrypt.activate_key - encrypt.revoke_key - encrypt.rotate - encrypt.verify - encrypt.keys - encrypt.status - encrypt.blind_index Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. encrypt.load_key() - GUC restore and empty passphrase validation: - Save previous encrypt.key_version before decryption - Restore it in exception handler on failure - Reject empty string passphrase (not just NULL) 2. encrypt.verify() - sample_size validation: - Require sample_size > 0, raise invalid_parameter_value - Add attnum > 0 AND NOT attisdropped to pg_attribute lookup 3. encrypt.rotate() - batch_size validation: - Require batch_size > 0, raise invalid_parameter_value 4. encrypt.register_key() - concurrent registration race fix: - Move LOCK TABLE before MAX(key_version) computation - Prevents two concurrent registrations getting same key ID 5. MIGRATION.md - fix grant syntax: - Changed invalid "GRANT SELECT ON encrypt.keys()" to - "GRANT USAGE ON SCHEMA encrypt" + "GRANT EXECUTE ON FUNCTION" Applied consistently to: - column_encrypt--4.0.sql (fresh install) - column_encrypt--3.3--4.0.sql (upgrade) - column_encrypt--3.1--3.3.sql (v3.3 upgrade) Added regression tests for new validations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
psql pads column headers to match the separator width. The header " load_key" needs trailing space to match "----------". Root cause: expected output was hand-written without the trailing whitespace that psql actually produces. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The encrypt.key_version GUC is defined as INTEGER with min=1.
Setting it to empty string '' would fail because:
1. Empty string cannot be parsed as an integer
2. Even if it could, 0 would be below the min value of 1
Replace `set_config('encrypt.key_version', '', false)` with
`EXECUTE 'RESET encrypt.key_version'` which correctly restores
the GUC to its default value (1).
Applied consistently to:
- column_encrypt--4.0.sql (fresh install)
- column_encrypt--3.3--4.0.sql (upgrade)
- column_encrypt--3.1--3.3.sql (v3.3 upgrade)
Also added documentation note about binary protocol security:
- The C layer blocks binary protocol (col_enc_send/col_enc_recv)
- Error: "binary protocol is not supported for encrypted types"
- Cannot be regression-tested because COPY TO STDOUT doesn't
work inside PL/pgSQL blocks
Verified with Docker regression tests (PostgreSQL 16, 18).
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses 6 Copilot review items: 1. activate_key() concurrency race: - Added LOCK TABLE ... IN EXCLUSIVE MODE to serialize concurrent activations - Prevents non-deterministic unique constraint violations 2. rotate() NULL handling with current_setting: - Changed `<> 'on'` to `IS DISTINCT FROM 'on'` - Handles NULL when GUC is missing (missing_ok=true) 3. Upgrade script hard-coded schema in DO block: - 3.3--4.0.sql now uses dynamic schema lookup with pg_extension - Uses pg_catalog.format() for safe identifier quoting 4. GRANT IN SCHEMA hard-coded: - 3.1--3.3.sql now uses DO block with dynamic schema lookup - IN SCHEMA requires literal name, so dynamic SQL is needed 5. Deprecated wrappers used SET search_path TO public: - Changed to pg_catalog for SECURITY DEFINER safety - Even deprecated functions should follow best practices 6. Copilot guidance incorrect about schema auto-creation: - Clarified that PostgreSQL does NOT auto-create schema - Schema must be pre-created before CREATE EXTENSION Also updated CLAUDE.md with rules for: - Concurrency handling for global state changes - GUC NULL handling with IS DISTINCT FROM Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…d schema step to README v3.0--3.1.sql fixes: - Change SET search_path TO public → pg_catalog in all SECURITY DEFINER functions - Prefix all extension objects with @extschema@. for correct resolution - Affected functions: cipher_coverage_audit, cipher_key_usage_stats, cipher_metrics, cipher_start/process/run/pause/resume/cancel_rotation_job, cipher_rotation_jobs_status README.md: - Add step 6: CREATE SCHEMA IF NOT EXISTS encrypt (required before CREATE EXTENSION) - Renumber subsequent steps (7, 8) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…MIGRATION.md MIGRATION.md: - Replace misleading "cannot upgrade directly" with accurate explanation - PostgreSQL CAN chain intermediate scripts (3.1→3.3→4.0 in one command) - Clarify that staged migration is recommended for operational reasons - v3.3 deprecation period allows app code updates before v4.0 removes old API CLAUDE.md: - Add Documentation Rule #5: distinguish SQL upgrade mechanics from migration guidance - Do not write "cannot upgrade" when intermediate scripts exist .github/copilot-instructions.md: - Add "PostgreSQL Extension Upgrade Mechanics" section - Explain that Copilot should not flag staged migration as incorrect - Provide correct vs incorrect framing examples Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…l chunk size The Copilot review correctly identified that docs/tests implied per-call incremental batching when rotate() actually processes the entire column in one call, using batch_size only as the internal UPDATE chunk size. Fix approach: Update docs/tests to match code (not vice versa) because: - Code behavior is consistent and reasonable - Function comments already said "Re-encrypts all data" - Changing runtime behavior would be a breaking change README.md: - Remove misleading DO block example that implied incremental batching - Clarify batch_size controls "internal UPDATE chunk size" - Update API table description MIGRATION.md: - Change "Batch limit" comment to accurate description SQL scripts (4.0, 3.1--3.3, 3.3--4.0): - Update function comment to: "Re-encrypts entire column with the active key. batch_size controls internal UPDATE chunk size." Tests: - Update test comments to clarify the DO block demonstrates completion verification, not incremental per-call processing CLAUDE.md: - Add Documentation Rule #6 about precise API parameter semantics .github/copilot-instructions.md: - Add "API Semantics and Parameter Naming" section - Guide reviewers to check function bodies, not just signatures - Clarify when to fix docs vs code for semantic mismatches Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…te enforcement
Copilot correctly identified that privilege checks in rotate() and verify()
used session_user(), which ignores SET ROLE privilege reduction. Also, tests
used SET ROLE but tables were owned by superuser, so privilege checks weren't
actually validated.
Privilege check fix (4.0.sql, 3.1--3.3.sql, 3.3--4.0.sql):
- Add v_effective_role variable using the pattern:
COALESCE(NULLIF(NULLIF(current_setting('role', true), ''), 'none'), session_user())
- This honors SET ROLE if used, falls back to session_user otherwise
- Prevents escalation while honoring privilege reduction
Test fix (sql/column_encrypt.sql, expected/column_encrypt.out):
- Create test_enc_text, test_enc_bytea, test_batch_rotate under SET ROLE regress_user
- Tables now owned by regress_user, so privilege checks are actually tested
- Tests now validate real privilege enforcement behavior
Documentation fixes:
- README.md: Add encrypt. prefix to loaded_cipher_key_versions() for consistency
- CLAUDE.md: Add "Privilege Model for SECURITY DEFINER Functions" section
- copilot-instructions.md: Add PostgreSQL privilege semantics table and guidance
- column_encrypt_review.prompt.md: Add privilege semantics review heuristics
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
COALESCE, NULLIF, CASE, and CAST are SQL language constructs, not functions in pg_catalog. Prefixing them with pg_catalog. causes "function does not exist" errors. This fixes the CI failure in encrypt.rotate() and encrypt.verify() where the effective-role initialization was using pg_catalog.NULLIF and pg_catalog.COALESCE incorrectly. Updated documentation to prevent this class of error: - CLAUDE.md: Added section on SQL constructs vs pg_catalog functions - copilot-instructions.md: Added same guidance for AI reviewers - column_encrypt_review.prompt.md: Clarified in privilege pattern example Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PostgreSQL pads column headers with trailing spaces to align with the column width. The expected output was missing trailing spaces on the 'data' header in the encrypted_bytea test. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The upgrade script (3.3--4.0.sql) was missing one comment line in the rotate() function that exists in the fresh install (4.0.sql), causing the CI function parity check to fail. Also fixed expected test output whitespace for encrypted_bytea column. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| requires = 'pgcrypto' | ||
| module_pathname = '$libdir/column_encrypt' | ||
| relocatable = false | ||
| schema = encrypt |
There was a problem hiding this comment.
Setting schema = encrypt while relocatable = false means new installs are pinned to the encrypt schema, but this repo still ships legacy install scripts (e.g., column_encrypt--2.0.sql) that hardcode DROP ... public.*. If someone installs an older version (e.g., CREATE EXTENSION ... VERSION '2.0') under the new control file, those scripts can drop unrelated user objects in public. Consider updating legacy install scripts to use @extschema@ (or otherwise avoid cross-schema drops), or stop supporting creation of pre-schema versions with this distribution.
Suggested change
| schema = encrypt | |
| # schema = encrypt |
… paths Fix contradictory guidance in .github/copilot-instructions.md: - Removed incorrect statement that PostgreSQL auto-creates the schema - Clarified that schema must be pre-created (matches README, CI, and actual behavior) Added documentation about supported install paths: - Fresh installs: only v3.3 and v4.0 are supported - Legacy scripts (1.0, 2.0, 3.0, 3.1) are shipped for upgrade compatibility only - Legacy scripts contain hardcoded public.* references from an older model - CI upgrade tests verify upgrade paths, not fresh legacy installs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines
+111
to
+113
| -- Basic blind index generation | ||
| SELECT encrypt.blind_index('123-45-6789', 'blind-index-secret'); | ||
|
|
There was a problem hiding this comment.
Blind index is executed after RESET ROLE (superuser). That means the regression suite doesn't actually verify that encrypt.blind_index() is callable by column_encrypt_user and denied to unprivileged users (the main security hardening called out in the PR). Consider running at least one encrypt.blind_index() call under SET ROLE regress_user and adding a negative test under SET ROLE regress_unprivileged (or a has_function_privilege check) to ensure PUBLIC access is really revoked.
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
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.
Pull Request: Simplify API with v4.0 Release
Summary
This PR refactors column_encrypt to provide a cleaner, simpler API while maintaining full backward compatibility through a proper deprecation path. The extension is streamlined to focus on
its core mission: transparent column-level encryption with minimal ceremony.
Key changes:
What's New
v4.0 Clean API (encrypt schema)
Upgrade Path
v3.1 → v3.3 (deprecation) → v4.0 (clean)
What's Removed in v4.0
Security
Files Changed
Testing
Migration Guide
See MIGRATION.md for detailed instructions.
Quick migration:
-- Step 1: Upgrade to deprecation release
ALTER EXTENSION column_encrypt UPDATE TO '3.3';
-- Step 2: Update code to use encrypt.* API
-- Step 3: Upgrade to clean release
ALTER EXTENSION column_encrypt UPDATE TO '4.0';
Breaking Changes
Commits: