Repository navigation
Conversation
🟡 Tier B · Needs changes before merging
This change adds encrypted-attribute detection to database operator parsing and rejects operators targeting those fields. The check is added alongside the existing relationship-attribute handling in the shared Databases action.
Fix with agent prompt### Issue 1
src/Appwrite/Platform/Modules/Databases/Http/Databases/Action.php:116-117
**Only reject encrypted attributes when the value is an operator**
`parseOperators` also receives ordinary write payloads from Documents/Update, Upsert, and Bulk/Update, so this unconditional key check rejects a normal plaintext assignment to any encrypted attribute. Please limit the rejection to values that actually encode a recognized operator; otherwise existing encrypted fields become unwritable through these APIs.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 1
Reviewed |
There was a problem hiding this comment.
🟡 Tier B · 1 blocking finding to address. Summary
| if (isset($encryptedKeys[$key])) { | ||
| throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Attribute "' . $key . '" is encrypted and does not support string operators.'); |
There was a problem hiding this comment.
Only reject encrypted attributes when the value is an operator
parseOperators also receives ordinary write payloads from Documents/Update, Upsert, and Bulk/Update, so this unconditional key check rejects a normal plaintext assignment to any encrypted attribute. Please limit the rejection to values that actually encode a recognized operator; otherwise existing encrypted fields become unwritable through these APIs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Appwrite/Platform/Modules/Databases/Http/Databases/Action.php
Line: 116-117
Comment:
**Only reject encrypted attributes when the value is an operator**
`parseOperators` also receives ordinary write payloads from Documents/Update, Upsert, and Bulk/Update, so this unconditional key check rejects a normal plaintext assignment to any encrypted attribute. Please limit the rejection to values that actually encode a recognized operator; otherwise existing encrypted fields become unwritable through these APIs.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟠 Major · bug · Reply if this doesn't apply.
Fixes #14218
parseOperatorsalready skips relationship attributes via a$relationshipKeyslookup before processing operators. There was no equivalent guard for encrypted attributes.When a
stringReplace(or any string operator) arrives for a column markedencrypt: true, the operator passes through and Postgres runsREPLACE(COALESCE(col, ''), ...)directly on the stored ciphertext envelope — a JSON blob withdata,method,iv, andtagkeys. Replacing common hex characters in that envelope corrupts its structure; decryption then returnsfalseand the value is unrecoverable.The
encryptflag is already available on every attribute Document at the timeparseOperatorsruns (set inapp/init/database/filters.php).Fix: Collect encrypted attribute keys in the same loop that already collects relationship keys, then throw
GENERAL_ARGUMENT_INVALIDbefore any operator is parsed for those attributes — mirroring the existing relationship-key guard exactly.