Fix key routes for base64 public keys#244
Conversation
|
@naturedesk is attempting to deploy a commit to the Vezures Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe SDKs now route base64-like public keys through URL-safe key paths in Python and TypeScript. TypeScript error classification also adds the non-retryable ChangesPublic-key key routing
Admin configuration error classification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant KeysApi
participant KeyRouter
participant KeysHandler
Application->>KeysApi: request bundle, health, prekeys, or signed-prekey
KeysApi->>KeyRouter: construct keyPath(agentId, suffix)
KeyRouter->>KeysHandler: send URL-safe by-public-key route
KeysHandler-->>Application: return key operation response
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/python/src/tinyplace/api/keys.py`:
- Around line 45-50: Update _looks_like_base64_public_key to match the
TypeScript heuristic exactly: use an ASCII-only regex for the allowed base64
characters and permit zero to two trailing padding characters, while preserving
the existing minimum length and required marker checks. Move or add the re
import at module scope as needed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7a50927-0cff-4718-8f45-20753c786940
📒 Files selected for processing (6)
sdk/python/src/tinyplace/api/keys.pysdk/python/tests/test_api_namespaces.pysdk/typescript/src/api/keys.tssdk/typescript/src/errors.tssdk/typescript/tests/keys.test.tssdk/typescript/tests/tinyplace-error.test.ts
| def _looks_like_base64_public_key(value: str) -> bool: | ||
| if len(value) < 40 or not any(marker in value for marker in "+/="): | ||
| return False | ||
| return all(char.isalnum() or char in "+/=" for char in value) and ( | ||
| value.rstrip("=").count("=") == 0 | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align _looks_like_base64_public_key with the TypeScript heuristic for cross-language parity.
The Python validation is more permissive than the TypeScript looksLikeBase64PublicKey in two ways: char.isalnum() is Unicode-aware (vs ASCII-only [A-Za-z0-9]), and there's no limit on trailing = count (vs {0,2}). A 40+ char string with 3+ trailing = or Unicode letters would pass Python but fail TypeScript, causing the SDKs to route the same input differently — undermining the PR's cross-language routing goal.
Using a regex matching the TypeScript implementation closes both gaps:
🔧 Proposed fix for cross-language parity
def _looks_like_base64_public_key(value: str) -> bool:
if len(value) < 40 or not any(marker in value for marker in "+/="):
return False
- return all(char.isalnum() or char in "+/=" for char in value) and (
- value.rstrip("=").count("=") == 0
- )
+ import re
+ return bool(re.fullmatch(r"[A-Za-z0-9+/]+={0,2}", value))Move the import re to the top of the file if not already present.
📝 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.
| def _looks_like_base64_public_key(value: str) -> bool: | |
| if len(value) < 40 or not any(marker in value for marker in "+/="): | |
| return False | |
| return all(char.isalnum() or char in "+/=" for char in value) and ( | |
| value.rstrip("=").count("=") == 0 | |
| ) | |
| def _looks_like_base64_public_key(value: str) -> bool: | |
| if len(value) < 40 or not any(marker in value for marker in "+/="): | |
| return False | |
| import re | |
| return bool(re.fullmatch(r"[A-Za-z0-9+/]+={0,2}", value)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/python/src/tinyplace/api/keys.py` around lines 45 - 50, Update
_looks_like_base64_public_key to match the TypeScript heuristic exactly: use an
ASCII-only regex for the allowed base64 characters and permit zero to two
trailing padding characters, while preserving the existing minimum length and
required marker checks. Move or add the re import at module scope as needed.
Summary
/keys/by-public-key/<base64url>/...in the TypeScript and Python SDKs while preserving legacy/keys/<agentId>/...paths for handles/ids/,+, and padding are not embedded directly as path segmentsadmin approval is not configuredas a non-retryable operator/configuration error instead of a transient 503Verification
./node_modules/.bin/vitest run tests/keys.test.ts tests/tinyplace-error.test.ts./node_modules/.bin/tsc --noEmituv run --with pytest --with pytest-asyncio pytest -o addopts='' tests/test_api_namespaces.pyNotes
This is the SDK/client half of #213. The live backend still needs to expose and decode the matching
/keys/by-public-key/<base64url>/...route for full end-to-end resolution. For #243, this improves the SDK/operator error surface, but the actual payout recovery still requires backend admin approval configuration or platform support.Refs #213
Refs #243
Summary by CodeRabbit
New Features
admin_not_configurederror classification with guidance indicating that the issue is not retryable.Bug Fixes