feat(hub): add X25519 sealed-envelope relay for store-and-forward through taos.my#2034
feat(hub): add X25519 sealed-envelope relay for store-and-forward through taos.my#2034hognek wants to merge 3 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesHub sealed-envelope relay
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant account_proxy
participant taos_upstream
Client->>account_proxy: Submit relay drop or poll request
account_proxy->>account_proxy: Validate recipient and body
account_proxy->>taos_upstream: Forward relay action
taos_upstream-->>account_proxy: Return relay response
account_proxy-->>Client: Return status and response body
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| recipient_x25519_pub_hex=recipient_x25519_pub_hex) | ||
| outer["recipient"] = recipient | ||
| # Enforce max size: the JSON-serialised envelope must be ≤ 32 KB. | ||
| raw = json.dumps(outer, sort_keys=True, separators=(",", ":")) |
There was a problem hiding this comment.
WARNING: Envelope size limit is measured on the raw JSON, not the base64-encoded wire form the docstring promises.
The module docstring (line 23) states the limit is 32 KB (outer envelope base64-encoded), but here you measure len(json.dumps(...).encode("utf-8")). base64 inflates by ~33%, so a 32 KB JSON envelope serializes to ~42 KB on the wire — the documented limit is never actually enforced at the transport layer. Either measure the base64-encoded body the hub will actually receive, or correct the docstring to say the limit applies to the UTF-8 JSON representation.
Also raw.encode("utf-8") is computed twice (here and again inside the ValueError); compute it once into a variable.
| # never sees plaintext — it only inspects ``recipient`` for routing. | ||
|
|
||
| @router.post("/api/account/hub/relay/drop") | ||
| async def hub_relay_drop(request: Request): |
There was a problem hiding this comment.
WARNING: hub_relay_drop forwards the request body (including the in-body recipient) verbatim with no validation tying the recipient to the authenticated session.
Unlike hub_relay_poll (which validates recipient via _valid_rid and rejects bad input before forwarding), this endpoint relays the recipient field from the POST body straight to taos.my. A client authenticated as hub:alice can therefore drop a sealed envelope addressed to hub:bob (or any arbitrary rid). Even though the payload is sealed to Bob's X25519 key so Alice can't read it, this permits recipient-address spam / mailbox flooding and bypasses the routing restriction that the poll side enforces. Consider validating the in-body recipient against _valid_rid and/or asserting it matches the session identity before forwarding.
| assert r.status_code == 200 | ||
| assert "taos_session" not in captured.get("cookie", "") | ||
| assert captured.get("cookie", "") == "" | ||
|
|
There was a problem hiding this comment.
SUGGESTION: The PR description notes these 3 new account-proxy tests "require full app fixture (pre-existing conftest hang on this machine)" and were not actually run. Confirm they execute in CI before merge — the inline monkeypatch/_FakeResp/_patch_upstream fixtures must be available in this file's scope, and a hang or skip here would leave the new relay routes effectively untested in the pipeline.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Incremental Check (since previous review commit
|
| File | Line | Previous | Status |
|---|---|---|---|
tinyagentos/hub/relay.py |
167 | Size limit measured on raw JSON, not base64; raw.encode computed twice |
RESOLVED — docstring now reads "JSON-serialised envelope"; raw_bytes computed once (lines 167-168) |
tinyagentos/routes/account_proxy.py |
377 | hub_relay_drop forwarded body without recipient validation |
RESOLVED — _valid_hub_recipient(recipient) check present at line 386 before forwarding |
tinyagentos/routes/account_proxy.py |
381 | Non-dict JSON body 500ed instead of 400 | RESOLVED — isinstance(payload, dict) guard at line 383; UnicodeDecodeError added to except at line 381; unit test test_hub_relay_drop_rejects_non_dict_body added |
All previously active Code Review Findings are resolved. No new issues introduced in the incremental change.
Previous Review Summaries (4 snapshots, latest commit 81d9f18)
Current summary above is authoritative. Previous snapshots are kept for context only.
Previous review (commit 81d9f18)
Status: No Issues Found | Recommendation: Merge
Files Reviewed (5 files)
tinyagentos/hub/relay.py- 0 issues (previous size-limit doc + double-encode finding RESOLVED)tinyagentos/routes/account_proxy.py- 0 issues (recipient validation + non-dict body guard both RESOLVED)tests/test_hub_relay.py- 0 issues (new, 13 passing tests)tests/test_routes_account_proxy.py- 0 issues (new relay forwarding/validation tests)tests/test_routes_github.py- 0 issues (test mock fix, unrelated to relay feature)
Previous Findings — Re-verified / Resolved
| File | Line | Previous | Status |
|---|---|---|---|
tinyagentos/hub/relay.py |
167 | Size limit measured on raw JSON, not base64; raw.encode computed twice | RESOLVED — docstring corrected to “JSON-serialised outer envelope, UTF-8 encoded”; raw_bytes computed once |
tinyagentos/routes/account_proxy.py |
377 | hub_relay_drop forwarded body without recipient validation |
RESOLVED — _valid_hub_recipient(recipient) check now present before forwarding |
tinyagentos/routes/account_proxy.py |
381 | Non-dict JSON body 500ed instead of 400 | RESOLVED — isinstance(payload, dict) guard added; UnicodeDecodeError added to except clause |
tests/test_routes_account_proxy.py |
624 | New relay tests not executed locally | INFO — pre-existing CI-confidence note; not a code defect. All relay tests now structurally present and the PR confirms they pass under Python 3.11. |
Previous review (commit 7d45621)
Status: No Issues Found | Recommendation: Merge
Files Reviewed (2 files)
tinyagentos/routes/account_proxy.py- 0 issues (previously reported non-dict body 500 and missing recipient validation both RESOLVED in this commit)tests/test_routes_account_proxy.py- 0 issues (newtest_hub_relay_drop_rejects_non_dict_bodyadded covering the fix)
Previous Findings — Re-verified / Resolved
| File | Line | Previous | Status |
|---|---|---|---|
tinyagentos/routes/account_proxy.py |
383-384 | Non-dict JSON body 500ed instead of 400 | RESOLVED — isinstance(payload, dict) guard added before .get(); UnicodeDecodeError also added to the except clause |
tinyagentos/routes/account_proxy.py |
386 | hub_relay_drop forwarded without recipient validation |
RESOLVED — _valid_hub_recipient(recipient) check now present before forwarding |
tinyagentos/hub/relay.py |
167 | Envelope size limit measured on raw JSON, not base64 wire form | Out of scope for this incremental diff (unchanged file); carried forward for tracking |
Previous review (commit 1faa15b)
Status: 2 Issues Found | Recommendation: Address before merge
Overview
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 2 |
| SUGGESTION | 0 |
Issue Details (click to expand)
WARNING
| File | Line | Issue |
|---|---|---|
tinyagentos/routes/account_proxy.py |
381 | Non-dict JSON body (array/string/scalar) passes json.loads but payload.get(...) raises AttributeError, uncaught by the (json.JSONDecodeError, TypeError) clause, so the request 500s instead of returning 400; mirror the isinstance(payload, dict) guard used in _validate_subdomain_name |
tinyagentos/routes/account_proxy.py |
377 | hub_relay_drop now validates _valid_hub_recipient before forwarding — RE-VERIFIED: previously reported missing validation is fixed |
Files Reviewed (4 files)
tinyagentos/hub/relay.py- 0 issues (previous size-limit docstring / double-encode WARNING resolved in this commit)tinyagentos/routes/account_proxy.py- 1 new issue (non-dict body 500), 1 re-verified resolvedtests/test_routes_account_proxy.py- 0 new (prior SUGGESTION about unrun tests remains; newtest_hub_relay_drop_rejects_invalid_recipientadded and is structurally identical to passing tests)tests/test_hub_relay.py- 0 issues
Fix these issues in Kilo Cloud
Previous review (commit c47d898)
Status: 3 Issues Found | Recommendation: Address before merge
Overview
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 2 |
| SUGGESTION | 1 |
Issue Details (click to expand)
WARNING
| File | Line | Issue |
|---|---|---|
tinyagentos/hub/relay.py |
167 | Envelope size limit measured on raw JSON, not base64 wire form promised by docstring; encode computed twice |
tinyagentos/routes/account_proxy.py |
367 | hub_relay_drop forwards in-body recipient verbatim with no session/rid validation (mailbox spam risk) |
SUGGESTION
| File | Line | Issue |
|---|---|---|
tests/test_routes_account_proxy.py |
624 | New account-proxy relay tests reportedly not run (conftest hang); verify they pass in CI |
Files Reviewed (4 files)
tinyagentos/hub/relay.py- 1 issuetinyagentos/routes/account_proxy.py- 1 issuetests/test_routes_account_proxy.py- 1 issuetests/test_hub_relay.py- 0 issues
Reviewed by hy3:free · Input: 36.4K · Output: 2.6K · Cached: 159.6K
|
Thanks - this builds nicely on the #2025 peer channel. Fold list before merge (test 3.13 is also failing, likely the unrun tests below): MUST-FIX (security):
Then: Once the two security items are folded and CI is green I will re-review and merge. |
| body = await request.body() | ||
| try: | ||
| payload = json.loads(body) | ||
| recipient = str(payload.get("recipient", "")) |
There was a problem hiding this comment.
WARNING: A JSON body that parses to a non-dict (e.g. a JSON array [1,2,3], a string, or a number) passes json.loads but then payload.get(...) raises AttributeError, which is not caught by the except (json.JSONDecodeError, TypeError) clause — so the request 500s instead of returning 400.
The sibling validator _validate_subdomain_name already guards with if not isinstance(payload, dict). Mirror that here: after parsing, reject non-dict bodies before calling .get.
| recipient = str(payload.get("recipient", "")) | |
| if not isinstance(payload, dict): | |
| return JSONResponse({"error": "invalid body"}, status_code=400) | |
| recipient = str(payload.get("recipient", "")) |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
All jaylfc MUST-FIX items from the 09:53 review are now addressed: 1. relay.py:167 — docstring/limit mismatch → Fixed in ed89178: docstring updated to "JSON-serialised outer envelope, UTF-8 encoded" to match code behavior. 2. account_proxy.py:377 — missing recipient validation → Fixed in ed89178: 3. test routes 3 new tests not run → Fixed in 81d9f18:
Kilo Code Review: All 4 findings (2 WARNING, 1 SUGGESTION, 1 WARNING-N/A) addressed in these same commits. Test matrix requires workflow approval for re-run (fork CI policy). |
…ough taos.my Add hub/relay.py module with X25519 ECDH+HKDF→ChaCha20-Poly1305 seal/unseal, envelope building with size limits (32KB), and canonical-JSON serialization. Add account proxy forwarding for POST /api/account/hub/relay/drop and GET /api/account/hub/relay/poll with recipient validation. Implements cross-user collab A3 (issue jaylfc#2015): the hub sealed-envelope relay for T1 transport. Hub never sees plaintext; inner payload is Ed25519-signed then X25519-sealed. Tests: 13/13 pass — round-trip, key mismatch, tamper detection, size limit, unicode, canonical serialization, base64 url-safety. Account proxy route tests added (3 new) but require full app fixture (pre-existing conftest hang on this machine; structurally identical to existing passing account-proxy hub tests).
- Add _valid_hub_recipient() for hub:username format validation - Validate recipient in hub_relay_drop before forwarding (was bypassed) - Fix hub_relay_poll to use hub-specific validator (was using generic _valid_rid which rejects colons in hub usernames) - Correct envelope size docstring: base64-encoded → UTF-8 JSON - Compute raw_bytes once instead of double-encoding in build_envelope - Add test_hub_relay_drop_rejects_invalid_recipient test All 4 relay tests pass (Python 3.11).
Add isinstance(payload, dict) check after json.loads() to prevent AttributeError on non-dict bodies (arrays, strings, numbers). Also add test_hub_relay_drop_rejects_non_dict_body test.
2 participants
Summary
Implements cross-user collab A3 (issue #2015): the hub sealed-envelope relay for T1 transport on taos.my.
What's added
tinyagentos/hub/relay.py— X25519 envelope sealing module:seal()/unseal()for keypair-based encrypt/decryptbuild_envelope()— wraps a sealed payload in a hub-routable outer envelope with recipient field and size enforcement (≤32KB)canonicalize()— canonical JSON serialization for Ed25519 signing (design doc section 2)tinyagentos/routes/account_proxy.py— hub relay forwarding:_ACTIONSentries:hub_relay_drop(POST) andhub_relay_poll(GET)POST /api/account/hub/relay/drop— forwards sealed envelope to taos.myGET /api/account/hub/relay/poll?recipient=...— polls for queued envelopes with recipient validationDesign
The hub never sees plaintext. Nodes seal inner (Ed25519-signed) payloads to the recipient's X25519 public key. The outer envelope only exposes
recipientfor routing andcreated_atfor TTL expiry. Recipient nodes poll and unseal locally.Tests
tests/test_hub_relay.py— 13 tests (all passing):tests/test_routes_account_proxy.py— 3 new tests added:References
docs/design/cross-user-collaboration.md(sections 6, 7)Summary by CodeRabbit
New Features
Tests