Conversation
Greptile SummaryThis PR adds end-to-end encryption support: an
Confidence Score: 4/5Safe to merge after adding key-length validation; all transport paths wire the key correctly. One P1 finding: missing 32-byte length check for both the str and bytes key inputs allows wrong-length keys to bypass client-side validation and produce opaque server errors. Everything else — enum definition, mapper serialisation/deserialisation, header injection across all four transport paths, public API export, and test coverage — is correct. src/s2_sdk/_validators.py and the bytes branch in src/s2_sdk/_ops.py (lines 646-647) both need a 32-byte length guard. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Caller calls basin.stream with key] --> B{key type?}
B -->|str| C[validate_encryption_key: check base64 format]
B -->|bytes| D[base64.b64encode the raw bytes]
B -->|None| E[no key stored]
C --> F[S2Stream created with b64 key]
D --> F
E --> G[S2Stream created without key]
F --> H[_request_headers merges key into HTTP headers]
H --> I[append unary POST]
H --> J[read unary GET]
H --> K[append session streaming POST]
H --> L[read session streaming GET]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/s2_sdk/_validators.py
Line: 57-61
Comment:
**Missing key-length validation for both `str` and `bytes` inputs**
`validate_encryption_key` confirms the string is valid base64 but never checks that it decodes to exactly 32 bytes (required for both AEGIS-256 and AES-256-GCM). A caller who passes a valid base64 string that encodes a 16-byte key will clear client-side validation, have the raw header forwarded, and then receive an opaque server rejection instead of a clear SDK error.
The same gap exists for the `bytes` branch in `_ops.py`: any-length `bytes` is silently encoded and sent. Adding `len(decoded) != 32` / `len(encryption_key) != 32` guards to both paths would surface this immediately with a meaningful `S2ClientError`.
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "polishes" | Re-trigger Greptile |
ce20410 to
0b7215c
Compare
88c2cfe to
3771f7d
Compare
No description provided.