#5609 Encrypt sensitive Personally Identifiable Information (PII)

[eugeniojimenes]

Resolves #5609

Description

This encrypts sensitive personal data at rest using the built-in Active Record Encryption (encrypts).

Encrypted columns (all non-deterministic):

• Partners::Child#date_of_birth
• Partners::AuthorizedFamilyMember#date_of_birth
• Partners::Family#guardian_phone
• Partners::Profile#executive_director_email, #executive_director_phone, #primary_contact_email, #primary_contact_phone, #primary_contact_mobile, #pick_up_email, #pick_up_phone
• ProductDriveParticipant#phone, #email

The line is person vs organization: these all identify a specific individual.

Out of scope: contact details that belong to an organization rather than a person (organizations.email, partners.email, donation_sites.email, vendors.email), address fields, and free text comments. The Devise users.email is personal data, but it is the login credential and would need deterministic encryption, which touches the auth path, the uniqueness validation and the admin email search. Discussed in the review thread.

Alternatives and tradeoffs:

• Non-deterministic (the more secure option) was chosen because none of these columns are queried by value, validated for uniqueness, or searched.
• Encryption makes date_of_birth impossible to sort in SQL, because the stored ciphertext does not follow date order (this is also true for deterministic encryption). Instead of removing the "sort children by date of birth" feature, the controller sorts the decrypted value in Ruby. This is safe because that page is not paginated and loads only one partner's children.
• date_of_birth was changed from date to text so the column can hold the ciphertext. The models re-declare attribute :date_of_birth, :date so the value still behaves as a date in forms, JSON and views. The conversion pins its format with using: "to_char(date_of_birth, 'YYYY-MM-DD')": a bare change_column serializes the date with whatever DateStyle the server happens to run, and the rewrite is in place and irreversible.
• The Partners::Profile columns needed no migration, since they are already string columns.
• Existing rows are re-encrypted in place by a data migration (find_in_batches + encrypt, with disable_ddl_transaction! and a small throttle). It skips rows that are already encrypted, so a run that fails halfway can be re-run and only does the rows that are left.

Key management: two keys (primary_key and key_derivation_salt) are read from the environment in every environment. Development and test use committed non-production keys, in .env.development and .env.test. Production and staging read them from the real environment and refuse to boot if they are missing.

Dependencies: none. This uses the built-in Rails encryption, no new gems.

Deploy notes:

1. Set AR_ENCRYPTION_PRIMARY_KEY and AR_ENCRYPTION_KEY_DERIVATION_SALT in production and staging (generate them with bin/rails db:encryption:init). Losing these keys means the encrypted data cannot be recovered.
2. The backfill migration re-encrypts existing rows. Existing rows keep reading correctly during the rollout thanks to support_unencrypted_data: true.
3. ⚠️ Follow-up: support_unencrypted_data is set to true for now, so reads of rows that are not yet backfilled do not raise an error. After the production backfill is done, a follow-up should set it to false to fully enforce encryption.

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• Model specs for the five models check that each value is saved as ciphertext and still reads back correctly. The date of birth also keeps its date type.
• A request spec checks that the children page sorts by date of birth in both directions, with empty dates last.
• spec/migrations/encrypt_dates_of_birth_spec.rb rewinds the schema to date, seeds legacy plaintext rows with raw SQL, runs both migrations against real Postgres, and checks that no date is lost. It covers leap days and ambiguous dates, and it asserts the result is identical under a non-ISO DateStyle.
• spec/migrations/backfill_encrypted_pii_spec.rb checks the backfill encrypts plaintext rows across all five models, leaves already-encrypted rows byte for byte unchanged, and picks up where a failed run stopped.
• The PDF and profile request specs pass, which is the path where the partner profile contact fields are rendered.

Note: when I ran the full suite locally, one system test failed (kit_system_spec.rb, the duplicate items modal from #5538). It is unrelated to this change and looks like an existing flaky test. Rubocop, Brakeman and factory_bot:lint all pass.

[dorner]

the Devise users.email

is there a reason this wasn't covered?

[dorner]

Have you confirmed that this won't result in any data loss? The date converts cleanly?

[eugeniojimenes]

Wrote a spec for it: spec/migrations/encrypt_dates_of_birth_spec.rb. It puts date_of_birth back to date, seeds plaintext rows with raw SQL, runs both migrations against real Postgres, and checks the dates come back unchanged. They do, so no data loss.

But it exposed a hidden dependency: a plain change_column ... :text serializes the date with whatever DateStyle the server has. The default is ISO, so we get 1990-03-04 and it works. Under SET DateStyle TO 'SQL, MDY' the spec fails: is 03/04/1990 April 3rd or March 4th? Ruby reads it day first and encrypts the wrong date. 02/29 and 12/31 have no valid month reading, so they end up nil.

Fixed by pinning the format in the migration: using: "to_char(date_of_birth, 'YYYY-MM-DD')". That ignores DateStyle, and the spec passes either way.

[dorner]

Can we find only non-encrypted rows to handle the case where a run might fail?

[eugeniojimenes]

You're right, and my "idempotent" comment was wrong: idempotent in the value, not in the work. record.encrypt writes unconditionally (it just calls update_columns), and our encryption is non-deterministic, so re-encrypting an already-encrypted row rewrites it with new ciphertext. Same value, wasted write. A failed run redid the whole table.

There's no way to filter this in SQL. Ciphertext is opaque to Postgres, and Rails has no scope for it (extend_queries only helps deterministic attributes, and ours are all non-deterministic). Matching the payload prefix with NOT LIKE '{"p":%' would work, but it hardcodes Rails' internal JSON format into a migration.

So I used the public API instead, skipping the row before the write:

batch.reject { |record| encrypted?(record) }.each(&:encrypt)

encrypted? uses record.encrypted_attribute?, and treats a blank value as done. We still read every row, which is one sequential scan, but we only write the ones that need it. That's the expensive part (WAL, replication), so a re-run after a failure only does what's left.

Covered in spec/migrations/backfill_encrypted_pii_spec.rb, including already-encrypted rows staying byte for byte unchanged.

[dorner]

Maybe we should add this to .env.development instead?

[eugeniojimenes]

Done. The keys are in .env.development now, and the initializer is a plain ENV.fetch with no environment branch and no hardcoded values.

One catch: .env.development alone isn't enough. dotenv doesn't load it in the test environment (it loads .env.test instead, see dotenv/rails.rb), and CI never creates a .env, so the suite wouldn't boot. So I added .env.test too, with its own key pair. Both are throwaway non-production keys, and .gitignore only covers .env and .env.production, so they're safe to commit.

Checked all three paths:

• development reads .env.development
• test reads .env.test, and the suite passes
• production with no keys set fails the boot with KeyError: key not found: "AR_ENCRYPTION_PRIMARY_KEY", instead of starting up and silently writing data nobody can read back

[eugeniojimenes]

the Devise users.email

is there a reason this wasn't covered?

You're right, "org data isn't personal" doesn't apply to users.email. The real reason it's not in this PR is cost: it's the login and it has a unique index, so it needs deterministic encryption, which pulls in:

• validates :email, uniqueness: {case_sensitive: false} emits LOWER(email), meaningless on ciphertext → becomes encrypts :email, deterministic: true, downcase: true plus a plain uniqueness: true.
• users_controller.rb:35 where("LOWER(email) = ?") → find_by(email:).
• scope :search_email uses LIKE '%query%'; no encryption supports substring match in SQL, so it's either exact match or decrypt-and-filter in Ruby (super-admin-only screen).
• extend_queries = true during the migration window, otherwise not-yet-backfilled users can't log in.

A bad deploy here locks everyone out, super admins included, so I'd rather do it as its own PR with a staged rollout. If separate, account_requests.name/.email belong in the same batch: also a named person, and its uniqueness validation plus cross-checks against User/Organization force deterministic encryption for the same reasons.

While we're drawing the line, two more scope calls for you:

• users.current_sign_in_ip/.last_sign_in_ip: IPs are personal data under GDPR; they'd need the same inet → text treatment as date_of_birth. In or out?
• Person-name columns (children.first_name/.last_name, families.guardian_*_name, authorized_family_members names, product_drive_participants.contact_name, partner_profiles.*_name, users.name) are still plaintext next to their now-encrypted phone/email. Names drive SQL sort/search on the children index, so the cost is the same tier as users.email. Deferred batch, or out of scope?

[eugeniojimenes]

NOTE: I also fixed a gap I'd missed here (the change is already in my latest commit)

While addressing the review I re-checked the whole schema with the same "named individual" test. The partner_profiles table stores contact details of specific people at the agency (the executive_director_*, primary_contact_* and pick_up_* emails and phones), which puts them in the same category as the product drive participants.

They are encrypted now. No migration was needed since they are already string columns, and nothing queries or uniqueness-validates them, so they stay non-deterministic like the rest. Partners::Profile is in the backfill, and the PDF and profile request specs pass. I also updated the PR description with the full column list.