feat(core): add @EncryptedAttribute decorator for transparent column encryption#18157
feat(core): add @EncryptedAttribute decorator for transparent column encryption#18157nullxx wants to merge 2 commits intosequelize:mainfrom
Conversation
…encryption Add a new @EncryptedAttribute decorator that provides transparent at-rest encryption for model attributes using a pluggable CipherStrategy interface. - Aes256GcmStrategy (default): AES-256-GCM with random IV + auth tag - Aes256CbcStrategy: AES-256-CBC with random IV - Versioned ciphertext format (version byte prefix) for future migration - Uses registerModelAttributeOptions with get/set (no global hooks) - Supports multiple encrypted attributes per model without conflicts - Strict key validation (32-byte Buffer or 64-char hex string) - 22 unit tests covering round-trip, strategies, validation, and edge cases
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a legacy Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Code
participant Model as Model Instance
participant Decorator as EncryptedAttribute<br/>(getter/setter)
participant Serializer as Serialization<br/>System
participant Strategy as CipherStrategy
participant DB as Database
rect rgba(100,200,100,0.5)
Note over User,DB: SET Operation (Encrypt)
User->>Model: set attribute = value
Model->>Decorator: setter(value)
Decorator->>Serializer: serialize(value, dataType)
Serializer-->>Decorator: plaintext Buffer
Decorator->>Strategy: encrypt(plaintext, key)
Strategy-->>Decorator: EncryptionResult { data }
Decorator->>DB: store encrypted BLOB
end
rect rgba(100,150,200,0.5)
Note over User,DB: GET Operation (Decrypt)
User->>Model: read attribute
Model->>Decorator: getter()
Decorator->>DB: fetch encrypted BLOB
DB-->>Decorator: packed ciphertext
Decorator->>Strategy: decrypt(packed, key)
Strategy-->>Decorator: plaintext Buffer
Decorator->>Serializer: deserialize(plaintext, dataType)
Serializer-->>Decorator: value
Decorator-->>Model: return value
Model-->>User: value
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.4)packages/core/src/decorators/legacy/encrypted-attribute.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m packages/core/test/unit/decorators/encrypted-attribute.test.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nullxx
changed the title
- Fix prettier formatting - Add missing JSDoc @param declarations on serialise() and getDataTypeId() - Replace for-loop with Buffer.map in test XOR strategy - Remove unused InferCreationAttributes import
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/decorators/legacy/encrypted-attribute.ts`:
- Around line 181-182: Add missing JSDoc `@param` and `@returns` for the functions
serialise(value: unknown): Buffer and getDataTypeId(value: unknown): number so
the linter rule jsdoc/require-param is satisfied; update the comment blocks
above serialise and getDataTypeId to describe the parameter "value" and the
return type (Buffer for serialise, numeric data-type id for getDataTypeId) and
include a short description for each function. Ensure the param name matches the
function signature ("value") and the `@returns` describes the returned value to
satisfy the linter.
- Around line 452-456: The custom getter is called without injecting the
decrypted value into the instance, so userGet (called in the encrypted-attribute
decorator) still reads ciphertext via getDataValue(property); before calling
userGet.call(this) set the instance's data for that property to the decrypted
value (e.g., this.setDataValue(property, value)), then call userGet and
afterwards restore the original encrypted value to avoid mutating state
permanently; reference the symbols userGet, value, property, getDataValue and
setDataValue when making the change.
- Around line 467-470: The setter currently accepts null/undefined and persists
null unconditionally, bypassing the decorator's allowNull policy; update the
setter in encrypted-attribute.ts (the function that calls
this.setDataValue(propertyName, ...)) to check the decorator option (e.g.,
options.allowNull or the decorator's allowNull flag) when value == null: if
allowNull is true, continue to call this.setDataValue(propertyName as any, null
as any); otherwise throw a descriptive TypeError (or ValidationError)
immediately to enforce the decorator-level contract instead of persisting null.
- Around line 239-241: Update the deserializer branch that checks logicalType so
DATE remains mapped to a Date (return (buf: Buffer) => new
Date(buf.toString())), but change DATEONLY and TIME to return strings (return
(buf: Buffer) => buf.toString()). Locate the conditional that uses logicalType
=== 'DATE' || logicalType === 'DATEONLY' || logicalType === 'TIME' and split it
so 'DATE' uses new Date(...) while 'DATEONLY' and 'TIME' use buf.toString();
keep the same function/closure shape that accepts a Buffer and returns the
correctly typed value.
In `@packages/core/test/unit/decorators/encrypted-attribute.test.ts`:
- Line 2: The import statement currently brings in both InferAttributes and
InferCreationAttributes but InferCreationAttributes is unused and triggers the
linter; update the import in the test by removing the unused symbol
InferCreationAttributes so only InferAttributes is imported (i.e., change the
import that currently lists "InferAttributes, InferCreationAttributes" to only
include "InferAttributes").
- Around line 16-352: The suite mutates a shared Sequelize instance and model
registry; add test lifecycle hooks to isolate each test by creating/resetting
and tearing down the Sequelize instance: add a beforeEach that initializes a
fresh sequelize, calls sequelize.sync({ force: true }) and ensures any models
used by the tests are (re)registered via sequelize.addModels where needed, and
add an afterEach that closes the sequelize (sequelize.close()) and clears
registered models to avoid cross-test leakage; update the top-level describe
block in encrypted-attribute.test.ts to include these beforeEach/afterEach hooks
so tests using sequelize, sequelize.addModels, and the
EncryptedAttribute-decorated Model classes run against a clean DB instance each
time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09d94387-ba9d-436b-8762-e7f504a25bce
📒 Files selected for processing (3)
packages/core/src/decorators/legacy/encrypted-attribute.tspackages/core/src/decorators/legacy/index.tspackages/core/test/unit/decorators/encrypted-attribute.test.ts
| if (logicalType === 'DATE' || logicalType === 'DATEONLY' || logicalType === 'TIME') { | ||
| return (buf: Buffer) => new Date(buf.toString()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify expected runtime types for DATEONLY/TIME in Sequelize data type definitions.
rg -n "class DATEONLY|class TIME|DATEONLY|TIME|AcceptedDate" packages/core/src/abstract-dialect/data-types.ts -C4Repository: sequelize/sequelize
Length of output: 1790
🌐 Web query:
Sequelize DATEONLY TIME data type deserialization canonical types
💡 Result:
Canonical JS types (what you should expect in Sequelize)
DataTypes.DATEONLYis canonically a string inYYYY-MM-DDform (not aDate). Internally, Sequelize normalizes/binds values by formatting them with UTC semantics (dayjs.utc(...).format('YYYY-MM-DD')) when you set the attribute / send it to the DB. [1]DataTypes.TIMEis canonically a string containing a time-of-day (typicallyHH:mm:ssand possibly fractional seconds depending on dialect + precision). Sequelize mainly defines the SQL type/precision; it does not (in core) coerce it into a JSDate. [2]
Deserialization (DB → JS) behavior
- When Sequelize reads values back, the value first may be parsed at the dialect level (via a dialect-registered parser), and then—if the query/attribute has an explicit Sequelize DataType—Sequelize calls the DataType’s
parseDatabaseValue. This behavior is described in Sequelize’s “Extending Data Types” docs. [3] DATEONLY’s class overridessanitize/toBindableValue(for normalization & binding), but it does not overrideparseDatabaseValuein core—so what you get back is typically whatever the driver/dialect parser returns (commonly already a string). [1][3]- A common “gotcha” is timezone shifting in some dialect/driver combinations (notably MSSQL historically), where a stored
DATElike2019-06-01could be returned as2019-05-31due to unintended UTC/local conversion. [4]
Practical takeaway
- If you want date-only / time-only values with stable semantics across timezones, treat them as opaque strings (
YYYY-MM-DD,HH:mm:ss[.SSS]) at your application boundary, and (if needed) use a dialect type parser / custom datatype parsing hook to force that behavior consistently. [3][4]
Sources:
[1] Sequelize v7 DATEONLY implementation (UTC formatting, string normalization) https://raw.githubusercontent.com/sequelize/sequelize/7a2d9317988d1ac5a80279687e75c35baf373526/packages/core/src/abstract-dialect/data-types.ts (snippet shown in search result)
[2] Sequelize v7 TIME datatype definition (precision + SQL type) https://raw.githubusercontent.com/sequelize/sequelize/7a2d9317988d1ac5a80279687e75c35baf373526/packages/core/src/abstract-dialect/data-types.ts (snippet shown in search result)
[3] Sequelize docs: Extending / custom data types (parseDatabaseValue, dialect parser notes) https://sequelize.org/docs/v7/other-topics/extending-data-types/
[4] Sequelize issue: MSSQL DATEONLY timezone conversion bug report #10982
🌐 Web query:
Sequelize DATEONLY TIME expected return type string or date
💡 Result:
In Sequelize, the expected JS return type differs by datatype:
-
DataTypes.DATEONLY: string, formatted as'YYYY-MM-DD'(date-only, no time). (sequelize.org)- Sequelize may accept various “date-like” inputs when setting the value, but when you read it back it’s intended to be the normalized date string. (sequelize.org)
-
DataTypes.TIME: string (a time-only value; format is dialect/driver dependent but conceptually ISO-8601 time). (sequelize.org)
If you’re seeing a Date object (or an ISO datetime like 1970-01-01T...Z) for a TIME column, that’s typically coming from the dialect driver / parsing behavior rather than Sequelize’s intended TIME => string mapping. (sequelize.org)
Citations:
- 1: https://sequelize.org/api/v7/classes/_sequelize_core.index._internal_.dateonly
- 2: https://sequelize.org/api/v7/classes/_sequelize_core.index._internal_.dateonly
- 3: https://sequelize.org/api/v7/classes/_sequelize_core.index._internal_.time
- 4: https://sequelize.org/api/v7/classes/_sequelize_core.index._internal_.time
Map DATEONLY and TIME to strings instead of Date objects.
The canonical types in Sequelize are:
DATE→DateobjectDATEONLY→ string (YYYY-MM-DD)TIME→ string (time-of-day)
Currently, all three are mapped to new Date(...), which breaks round-trips. Specifically, DATEONLY and TIME should deserialize to strings via buf.toString().
Proposed fix
- if (logicalType === 'DATE' || logicalType === 'DATEONLY' || logicalType === 'TIME') {
+ if (logicalType === 'DATE') {
return (buf: Buffer) => new Date(buf.toString());
}
+
+ if (logicalType === 'DATEONLY' || logicalType === 'TIME') {
+ return (buf: Buffer) => buf.toString();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (logicalType === 'DATE' || logicalType === 'DATEONLY' || logicalType === 'TIME') { | |
| return (buf: Buffer) => new Date(buf.toString()); | |
| } | |
| if (logicalType === 'DATE') { | |
| return (buf: Buffer) => new Date(buf.toString()); | |
| } | |
| if (logicalType === 'DATEONLY' || logicalType === 'TIME') { | |
| return (buf: Buffer) => buf.toString(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/decorators/legacy/encrypted-attribute.ts` around lines 239
- 241, Update the deserializer branch that checks logicalType so DATE remains
mapped to a Date (return (buf: Buffer) => new Date(buf.toString())), but change
DATEONLY and TIME to return strings (return (buf: Buffer) => buf.toString()).
Locate the conditional that uses logicalType === 'DATE' || logicalType ===
'DATEONLY' || logicalType === 'TIME' and split it so 'DATE' uses new Date(...)
while 'DATEONLY' and 'TIME' use buf.toString(); keep the same function/closure
shape that accepts a Buffer and returns the correctly typed value.
| // If the user provided a custom getter, call it with the decrypted value | ||
| // already set so getDataValue returns the decrypted form. | ||
| if (userGet) { | ||
| return userGet.call(this); | ||
| } |
There was a problem hiding this comment.
Custom getter receives ciphertext instead of decrypted value.
Lines 452-456 state decrypted data is exposed to userGet, but value is never injected before calling userGet.call(this). A custom getter using getDataValue(property) will still read encrypted bytes.
💡 Proposed fix
const decrypted = strategy.decrypt(raw, keyBuf);
const value = deserialise(decrypted);
@@
if (userGet) {
- return userGet.call(this);
+ const dataValues = (this as any).dataValues;
+ const previous = dataValues[propertyName];
+ dataValues[propertyName] = value;
+ try {
+ return userGet.call(this);
+ } finally {
+ dataValues[propertyName] = previous;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If the user provided a custom getter, call it with the decrypted value | |
| // already set so getDataValue returns the decrypted form. | |
| if (userGet) { | |
| return userGet.call(this); | |
| } | |
| // If the user provided a custom getter, call it with the decrypted value | |
| // already set so getDataValue returns the decrypted form. | |
| if (userGet) { | |
| const dataValues = (this as any).dataValues; | |
| const previous = dataValues[propertyName]; | |
| dataValues[propertyName] = value; | |
| try { | |
| return userGet.call(this); | |
| } finally { | |
| dataValues[propertyName] = previous; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/decorators/legacy/encrypted-attribute.ts` around lines 452
- 456, The custom getter is called without injecting the decrypted value into
the instance, so userGet (called in the encrypted-attribute decorator) still
reads ciphertext via getDataValue(property); before calling userGet.call(this)
set the instance's data for that property to the decrypted value (e.g.,
this.setDataValue(property, value)), then call userGet and afterwards restore
the original encrypted value to avoid mutating state permanently; reference the
symbols userGet, value, property, getDataValue and setDataValue when making the
change.
| if (value == null) { | ||
| this.setDataValue(propertyName as any, null as any); | ||
|
|
||
| return; |
There was a problem hiding this comment.
allowNull is not enforced in the setter.
Line 467 accepts null/undefined and persists null regardless of allowNull, which bypasses decorator-level intent and delays failure to later validation/database layers.
💡 Proposed fix
if (value == null) {
+ if (!allowNull) {
+ throw new TypeError(`@EncryptedAttribute: "${propertyName}" does not allow null values.`);
+ }
this.setDataValue(propertyName as any, null as any);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (value == null) { | |
| this.setDataValue(propertyName as any, null as any); | |
| return; | |
| if (value == null) { | |
| if (!allowNull) { | |
| throw new TypeError(`@EncryptedAttribute: "${propertyName}" does not allow null values.`); | |
| } | |
| this.setDataValue(propertyName as any, null as any); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/decorators/legacy/encrypted-attribute.ts` around lines 467
- 470, The setter currently accepts null/undefined and persists null
unconditionally, bypassing the decorator's allowNull policy; update the setter
in encrypted-attribute.ts (the function that calls
this.setDataValue(propertyName, ...)) to check the decorator option (e.g.,
options.allowNull or the decorator's allowNull flag) when value == null: if
allowNull is true, continue to call this.setDataValue(propertyName as any, null
as any); otherwise throw a descriptive TypeError (or ValidationError)
immediately to enforce the decorator-level contract instead of persisting null.
There was a problem hiding this comment.
Add test lifecycle hooks for isolation and guideline compliance.
This suite mutates shared Sequelize model registration without beforeEach/afterEach setup/teardown, which can cause inter-test coupling and violates the repository test lifecycle rule.
As per coding guidelines "Use beforeEach/afterEach to sync with { force: true } and close Sequelize instances in tests".
🧰 Tools
🪛 ESLint
[error] 277-277: Use a for-of loop instead of this for loop.
(unicorn/no-for-loop)
[error] 285-285: Use a for-of loop instead of this for loop.
(unicorn/no-for-loop)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/test/unit/decorators/encrypted-attribute.test.ts` around lines
16 - 352, The suite mutates a shared Sequelize instance and model registry; add
test lifecycle hooks to isolate each test by creating/resetting and tearing down
the Sequelize instance: add a beforeEach that initializes a fresh sequelize,
calls sequelize.sync({ force: true }) and ensures any models used by the tests
are (re)registered via sequelize.addModels where needed, and add an afterEach
that closes the sequelize (sequelize.close()) and clears registered models to
avoid cross-test leakage; update the top-level describe block in
encrypted-attribute.test.ts to include these beforeEach/afterEach hooks so tests
using sequelize, sequelize.addModels, and the EncryptedAttribute-decorated Model
classes run against a clean DB instance each time.
3 participants
Pull Request Checklist
Description of Changes
Closes #17299
Summary
Adds a new
@EncryptedAttributedecorator to@sequelize/core/decorators-legacythat provides transparent at-rest encryption for model attributes. The attribute is stored in the database as aBLOBcontaining a versioned packed ciphertext and is automatically decrypted on read, with the value coerced back to the declared logical DataType.Motivation
Sequelize currently has no built-in mechanism for encrypting sensitive model fields (SSNs, credit card numbers, PII, API keys, etc.). Users must manually implement encryption in hooks or getters/setters, which is error-prone and inconsistent. This decorator provides a standardised, secure-by-default solution that integrates natively with Sequelize's decorator infrastructure.
Design decisions
Strategy Pattern for cipher extensibility -- Instead of hardcoding a single algorithm, the decorator accepts a
CipherStrategyinterface. Two built-in strategies are provided:Aes256GcmStrategy(default) -- AES-256-GCM with random 12-byte IV + 16-byte auth tag. Provides both confidentiality and authenticity.Aes256CbcStrategy-- AES-256-CBC with random 16-byte IV. For environments where GCM is not available.Users can implement their own
CipherStrategy(e.g.chacha20-poly1305, envelope encryption with KMS, etc.) without any changes to Sequelize.Versioned ciphertext format -- Every packed ciphertext is prefixed with a version byte (
0x01), allowing the format to evolve in future versions without breaking existing encrypted data.Random IV per encryption -- Every
setoperation generates a fresh random IV viacrypto.randomBytes(). This is critical for GCM security (IV reuse with the same key completely breaks GCM confidentiality and authenticity).Native Sequelize integration via
get/set-- UsesregisterModelAttributeOptions()with customget/setfunctions instead of global model hooks. This ensures multiple@EncryptedAttributedecorators on the same model work independently without conflicts.Strict key validation -- Keys must be exactly 32 bytes (Buffer) or a 64-character hex string. The decorator throws eagerly at decoration time, not at runtime.
Usage example
Files changed
packages/core/src/decorators/legacy/encrypted-attribute.tspackages/core/src/decorators/legacy/index.tsexport * from './encrypted-attribute.js'packages/core/test/unit/decorators/encrypted-attribute.test.tsTest coverage
blobLengthoption respectedList of Breaking Changes
None. This is a purely additive feature with no changes to existing APIs.
Summary by CodeRabbit
New Features
Tests