Conversation
WalkthroughA shared Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 48-50: Add type hints to the jso helper to improve readability and
follow project guidelines: import typing.Any and annotate the function signature
as def jso(obj: Any) -> Any, keeping the implementation intact that calls
to_js(..., dict_converter=js.Object.fromEntries). Reference the existing jso
function and the to_js/js.Object.fromEntries usage so the maintainer knows
exactly where to add the import and signature change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8ba5736-4e7b-4857-b67c-f30e7ce5a9b3
📒 Files selected for processing (1)
src/worker.py
| def jso(obj): | ||
| """Utility to convert Python types to JS objects (Object.fromEntries for dicts).""" | ||
| return to_js(obj, create_pyproxies=False, dict_converter=js.Object.fromEntries) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nice fix for the Map vs Object issue!
This helper correctly addresses the root cause described in the PR — js.Object.fromEntries ensures Web Crypto APIs receive plain JS Objects instead of Maps.
One small improvement for maintainability: consider adding type hints. Since the function accepts various Python types and returns a JS proxy, something like:
from typing import Any
def jso(obj: Any) -> Any:
...This helps future contributors understand the function's flexibility at a glance. As per coding guidelines, "Review Python code for... proper type hints."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/worker.py` around lines 48 - 50, Add type hints to the jso helper to
improve readability and follow project guidelines: import typing.Any and
annotate the function signature as def jso(obj: Any) -> Any, keeping the
implementation intact that calls to_js(...,
dict_converter=js.Object.fromEntries). Reference the existing jso function and
the to_js/js.Object.fromEntries usage so the maintainer knows exactly where to
add the import and signature change.
|
This is a duplicate of #27 |
1 participant
Abstract
Hello, this PR has the goal to fix the
_import_aes_keysince the last PR seems to have broken the encryption. This addresses a critical regression in the authenticated encryption layer of our Cloudflare Python Worker, ensuring all PII and sensitive data can be successfully protected and retrieved.Current problem
The
js.crypto.subtle.importKeymethod (and other Web Crypto APIs) strictly requires a plain JavaScript Object for the algorithm parameter. However, by default, the Pyodideto_js()function converts Python dictionaries into JavaScript Map objects.When a Map is passed where an Object is expected, the internal JavaScript engine fails to find the required properties (like
.name), resulting in aNotSupportedError: Unrecognized key import algorithm "undefined" requested. This effectively broke all authenticated encryption paths in the application.Fix
The solution involves three main improvements:
jso()helper that ensures all Python-to-JS conversions for "dictionary-like" parameters usejs.Object.fromEntries. This guarantees that the Web Crypto API receives plain JS Objects.jso()utility. This significantly reduced the verbosity and "noise" of theto_jscalls.AES-GCM-beforeVsAfter.mp4
Fix for AES-GCM Encryption/Decryption
This PR fixes a regression in authenticated encryption handling in the Cloudflare Python Worker. The root cause was that Pyodide's
to_js()function converts Python dictionaries to JavaScript Map objects, but the Web Crypto API requires plain JavaScript Objects for algorithm parameters, causing aNotSupportedErrorwith "Unrecognized key import algorithm 'undefined'".Key Changes
New
jso()helper function: Introduced a lightweight utility that wrapsto_js()withdict_converter=js.Object.fromEntriesto ensure Python dictionaries are converted to plain JavaScript Objects (not Map objects) that Web Crypto APIs can properly consume.Centralized imports: Moved
import jsandfrom pyodide.ffi import to_jsto the top level of the module, removing redundant per-function imports.Refactored crypto functions: Updated three async functions to use the new
jso()helper:_import_aes_key(): Simplified key bytes, algorithm object, and usages array conversionencrypt_aes(): Cleaned up IV generation, algorithm object, and plaintext buffer handlingdecrypt_aes(): Simplified algorithm object and ciphertext buffer conversionImpact
The changes restore correct AES-GCM encryption and decryption functionality for protecting and retrieving PII and sensitive data in the authenticated encryption layer. The fix maintains backward compatibility with legacy XOR decryption while ensuring Web Crypto receives properly formatted parameters.