feat(protect): searchable encryption and protect schemas#96
feat(protect): searchable encryption and protect schemas#96calvinbrewer merged 32 commits intomainfrom
Conversation
Not working yet (needs Encrypt config support in Protect.js)
Add search support
| - Follow the prompts to indicate the type of version bump (patch, minor, major). | ||
| - The [GitHub Actions](./.github/workflows/) (or other CI pipeline) will handle the **publish** step to npm once your PR is merged and the changeset is committed to `main`. | ||
|
|
||
| ## Pre release process |
There was a problem hiding this comment.
Awesome, thanks for documenting this (and for wiring up pre releases)
README.md
Outdated
|
|
||
| > [!IMPORTANT] | ||
| > Searching, sorting, and filtering on encrypted data is only supported in PostgreSQL at the moment. | ||
| > Read more about [searching encrypted data](./docs/searchable-encryption.md). |
There was a problem hiding this comment.
I think that changing the phrasing around Postgres support makes it less clear which databases we do actually support. Previously it sounded like Postgres was the only database supported, but now it's ambiguous. Not blocking for this PR, but I think we'll want to clarify which databases we support/plan on supporting and for which features.
| // return sql`cs_ore_64_8_v1(${left}) < cs_ore_64_8_v1(${bindIfParam(right, left)})`; | ||
| // }; | ||
|
|
||
| const csMatch: BinaryOperator = (left: SQLWrapper, right: unknown): SQL => { |
There was a problem hiding this comment.
I think that these functions can be extracted into a package for a Drizzle integration. Using functions like csEq or cs.Eq would feel pretty similar to the typical Drizzle API and would mean that users wouldn't have to be concerned about calling EQL functions themselves most of the time.
Probably already on your list, but wanted to mention it just in case.
| </br> | ||
|
|
||
| [](https://github.com/cipherstash/protectjs/actions/workflows/tests.yml) | ||
| [](https://cipherstash.com) |
There was a problem hiding this comment.
Why remove this? I've been putting this on all our repos. Looks better than the non-transparent logo IMHO.
There was a problem hiding this comment.
Added it above the fold!
| ```ts | ||
| import { csTable, csColumn } from '@cipherstash/protect' | ||
|
|
||
| export const users = csTable('users', { |
There was a problem hiding this comment.
As I mentioned in our call, I think the term csTable is confusing because (to me at least) it feels like this is defining the table. But what it's actually doing is protecting it.
Given the name of the library is Protect.js, why don't we call this protect?
export const users = protect.table('users', {
email: protect.column('email'),
})There was a problem hiding this comment.
Although we've used protect to load the protect client. So how about this:
import { protected } from '@cipherstash/protect'
export const users = protected.table('users', {
email: protected.column('email'),
})There was a problem hiding this comment.
I see your point! Here are my thoughts:
- Using
csTableandcsColumnbring some CipherStash branding into the picture to tie Protect.js and CipherStash together. - Replicates building our your database schema, which in a way is kinda what we are doing. I actually really love how it replicates the Drizzle experience.
csTableis a much more JS way of doing things in this specific manner compared toprotected.table
There was a problem hiding this comment.
Another reason I really like the csTable and csColumn approach is because it is definitely more tied to the database than the core of Protect.js as it's bridging the gap between EQL, CipherStash, and the interface.
Another point on that is when we build out ORM specific examples. E.g.
This is how you do equality with Drizzle today
import { eq } from "drizzle-orm";
import { integer, pgTable, varchar } from "drizzle-orm/pg-core";
export const users = pgTable('users', {
id: integer(),
email: varchar('email')
})
db.select().from(table).where(eq(users.email, "test@example.com"));To maintain consistency with Drizzle, TypeORM, and any other ORM we interface with it'd look like this and be an awesome dev ex
import { csEq } from "@cipherstash/drizzle-orm"
import { csTable, csColumn } from "@cipherstash/protect"
export cost protectedUsers = csTable('users', {
email: csColumn('email').equality()
})
const searchTerm = protectClient.encrypt("test@example.com", {
table: protectedUsers,
column: protectedUsers.email
})
db.select().from(table).where(csEq(protectedUsers.email, searchTerm.data));| column: users.email, | ||
| table: users, |
There was a problem hiding this comment.
Given the schema already knows what column and table are being protected, can we set the EQL identity (ie. column and table) automatically for the user? Its a bit of a pain having to provide these values every time.
There was a problem hiding this comment.
I think being declarative in this instance is a good idea
There was a problem hiding this comment.
There could definitely be some room for improvement here but without major refactoring it'd be difficult to do that
| } | ||
| } | ||
|
|
||
| const csEq: BinaryOperator = (left: SQLWrapper, right: unknown): SQL => { |
There was a problem hiding this comment.
Naming things is hard but I wonder if the cs prefix will seem confusing to folks.
We all know that it stands for CipherStash but people are using the Protect library and the company that makes it might feel a bit abstract.
Another way to think about this: what is the difference in behaviour (or outcome) that this version of eq delivers over the standard one?
IMHO its safety/security.
Perhaps instead of csEq we call this secureEq or safeEq?
There was a problem hiding this comment.
We can thing about a Drizzle interface for Protect.js in a follow up feature 😎
docs/schema.md
Outdated
|
|
||
| You can chain these methods to your column to configure them in any combination. | ||
|
|
||
| ## Initializing the EQL client |
There was a problem hiding this comment.
Should this say "Protect client"?
| type AtLeastOneCsTable<T> = [T, ...T[]] | ||
| export const protect = async ( | ||
| ...tables: AtLeastOneCsTable<ProtectTable<ProtectTableColumn>> | ||
| ): Promise<ProtectClient> => { | ||
| if (!tables.length) { | ||
| throw new Error( | ||
| '[protect]: At least one csTable must be provided to initialize the protect client', | ||
| ) | ||
| } | ||
|
|
coderdan
left a comment
There was a problem hiding this comment.
This is a massive step forward. Well done.
I've left a number of comments that I think we should address quickly but I don't want them to block the merge. Noting that addressing the changes could well result in breaking changes.
This next version contains the following features and improvements. This will be a breaking change when released.
Check out the README for the usability changes.
The schema flow looks like this: