fix(#5549): harden Beacon x402 wallet/export fallbacks#5554
Conversation
github-actions
Bot
added
BCOS-L1
|
Security Review ✅ Expanded Beacon x402 hardening: dict() wrapper for sqlite3.Row access, contracts export tolerates uninitialized table, relay wallet lookup with coinbase_address. More robust than #5549. Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ |
jaxint
left a comment
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the contribution.
kekehanshujun
left a comment
kekehanshujun
left a comment
There was a problem hiding this comment.
I reviewed this as part of the #73 code review bounty.
Findings:
- The PR body uses
Closes #5549, but #5549 is an open PR, not the underlying bounty/bug issue. If this merges, the cross-reference is misleading and may confuse review/payment tracking. Please link the actual bug/bounty issue, or useRefs #5549if the intent is only to reference the prior PR. premium_contracts_export()now catches every exception from the contracts query and returns an empty successful export. That hides real database corruption/permission errors as “no contracts”, which makes operational failures silent. Please narrow the catch to the expected missing-table/missing-schema case, or return a generic 5xx for unexpected DB errors while still avoiding public exception disclosure.- This PR also modifies
node/server_proxy.pyandotc-bridge/otc_bridge.py, which are unrelated to the Beacon x402 export/wallet fallback described in the title. Splitting those hunks would reduce conflicts with #5555/#5556 and keep the Beacon fix reviewable.
I received RTC compensation for this review.
TJCurnutte
left a comment
TJCurnutte
left a comment
There was a problem hiding this comment.
Requesting changes because the relay wallet fallback still 500s on the same sqlite3.Row access pattern this PR is trying to harden.
Validated head 2a5af2101828ccc0fa105110f267ecea94e736d8 locally:
git diff --check origin/main...HEAD -- node/beacon_x402.py node/server_proxy.py otc-bridge/otc_bridge.py
python3 -B -m py_compile node/beacon_x402.py node/server_proxy.py otc-bridge/otc_bridge.pyThen I ran a focused Flask/temp-SQLite probe with conn.row_factory = sqlite3.Row, no native beacon_wallets row, and a matching relay_agents.coinbase_address row. The contracts export fallback added here worked, but the relay wallet lookup did not:
GET /api/agents/relay1/wallet -> 500
AttributeError: 'sqlite3.Row' object has no attribute 'get'
node/beacon_x402.py:257: if relay and relay.get("coinbase_address"):
GET /api/premium/contracts/export with no contracts table -> 200
{"contracts":[],"exported_at":...,"total":0}
So /api/agents/<id>/wallet still cannot return the relay wallet path whenever the DB returns sqlite3.Row. Please convert relay to a dict before .get(...) or use relay["coinbase_address"] after checking the column exists, and add a regression for the relay-agent fallback path.
jaxint
left a comment
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Great work on this PR. 🚀
|
Hi @TJCurnutte, could you take a look at this PR when you have a moment? Thanks! |
|
Duplicate of #5549 with scope-explosion. Adds unrelated changes to node/server_proxy.py and otc-bridge/otc_bridge.py and broadens the catch to except Exception. Per Codex forensic audit (2026-05-18), this is one of three near-identical duplicates of the same root cause. First-poster #5549 is the canonical fix. |
7 participants
Closes #5549