Bridge debugging Report #282
Conversation
|
@soamdesai-tfh is attempting to deploy a commit to the Worldcoin Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 253d109d51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| report: IDKitDebugReport | undefined, | ||
| ): T { | ||
| if ( | ||
| !report || |
There was a problem hiding this comment.
Remove raw debug payload when reports are disabled
When debug mode is off, createIDKitDebugReport() returns undefined, so this !report branch rethrows the original WASM BridgeRequestFailed error without calling deleteRawDebugPayload(). Since wasm_bindings.rs now attaches the pre-encryption request as error.debugPayload for bridge-create failures, any production bridge /request failure exposes that sensitive payload on the thrown error even though debug reporting is disabled; clean up debugPayload regardless of whether a report is created, or only attach it from WASM when debug is enabled.
Useful? React with 👍 / 👎.
Takaros999
left a comment
There was a problem hiding this comment.
high level, i believe we can avoid the log changed and simplify the debug report
once we nail this, you can add it to the kotlin and swift SDK as well
| cached_signal_hashes: CachedSignalHashes, | ||
| /// Request payload built by `IDKit` before bridge encryption/wrapping. | ||
| /// Stored unconditionally because Rust does not know whether JS debug mode is enabled. | ||
| debug_payload: serde_json::Value, |
There was a problem hiding this comment.
This is the request payload we sent to the bridge. We need to include the bridge response (if it exists as well)
| Other(Error), | ||
| } | ||
|
|
||
| fn bridge_request_failed(message: impl Into<String>, debug_payload: &serde_json::Value) -> Error { |
There was a problem hiding this comment.
im not very convinced adding the request payload into every single error will help here.
I would reduce it down to a single point, where if the debug flag is enabled we log the debug payload. Otherwise the RP can request it via getDebugReport
| }); | ||
| } | ||
|
|
||
| fn bridge_create_error_to_js(error: crate::Error) -> JsValue { |
| | "world_app_android" | ||
| | "unknown"; | ||
|
|
||
| export type IDKitDebugReport = { |
There was a problem hiding this comment.
Since this v1 i would avoid strong typing here, to start with you can try:
- request + response bridge payload (type: object)
- version
- package version is useful
- timestamps
- transport: keep it simple (
bridge | mini_app) - if we're inside a mini app, you can also add world app version + platform https://github.com/worldcoin/minikit-js/blob/main/packages/core/src/global.d.ts if
window.WorldAppis defined
Simplify the debug report and tighten its handling per review: - Capture the bridge response (create + poll) alongside the request via a new debug_response_payload on BridgeConnection, exposed through WASM as debugResponsePayload and surfaced as report.response_payload. - Stop threading the request payload into every bridge error: remove the Error::BridgeRequestFailed variant and the unconditional debugPayload attachment at the WASM boundary. The payload is now obtained at a single isDebug()-gated point (getBridgeDebugPayload), closing the off-state leak. - Flatten IDKitDebugReport to the loose v1 shape (version, package_version, timestamps, transport: bridge|mini_app, request/response payloads, mini_app from window.WorldApp); drop sha256 fingerprinting and the curated native log helpers. - Update the report on timeout/cancelled poll-loop exits for both bridge and invite-code paths. Follow-up hardening: - getDebugReport() short-circuits on !isDebug() so disabling debug stops stored-proof egress. - "failed" poll status only stamps response_received_at when a poll response actually arrived (matches the exception path). - Omit the rebuilt, non-correlatable proof_request.id from creation-error reports while keeping the real id on the live path. - Fix cloneValue JSON fallback to use "null" instead of invalid "undefined". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Drop the curated `request` summary object (mode/app_id/action/environment/ return_to/flags), the IDKitDebugRequestMode type, and requestModeFromConfig; every field is already carried by request_payload. - Guard getWasmDebugPayload/getWasmDebugResponsePayload behind isDebug() so debugPayload()/debugResponsePayload() no longer run on the default-off path (previously fired once per poll for all users). - Capture the HTTP status on a failed bridge poll instead of collapsing every non-2xx into connection_failed; surfaces at response_payload.poll.http_status. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A failed bridge POST /request previously folded the HTTP status and body into
an opaque error message string, leaving the report's response side unstructured
on the creation path (unlike the poll path, which now carries http_status).
Add an Error::BridgeRequestFailed { status, body } variant; the WASM boundary
attaches the namespaced { create: { http_status, body } } object as a single
debugResponsePayload property on the JS error, and request.ts lifts it as-is
into response_payload.create. This mirrors the existing create/poll namespacing
(bridge.rs success path + poll fix) and keeps the bridge-specific shape defined
in Rust rather than reconstructed in TS.
Raw body is stored (no "no error details" placeholder) so the structured field
is truthful: an empty body is reported as empty.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fb33bcb to
553736e
Compare
Restore the three console.debug calls in the native transport to their original full-payload form, per review feedback to avoid changing the existing logging. The full request/response payloads remain available through the structured debug report (getDebugReport), so no debug data is lost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| use std::str::FromStr; | ||
| #[cfg(feature = "ffi")] | ||
| use std::sync::Arc; | ||
| use std::sync::Mutex; |
There was a problem hiding this comment.
using a sync mutex will block the thread, in async rust we usually use mutex from tokio
| }); | ||
| } | ||
|
|
||
| fn bridge_create_error_to_js(error: crate::Error) -> JsValue { |
There was a problem hiding this comment.
what is the benefit of this? the debug report should have everything
| /// Request payload built by `IDKit` before bridge encryption/wrapping. | ||
| /// Stored unconditionally because Rust does not know whether JS debug mode is enabled. | ||
| debug_payload: serde_json::Value, | ||
| debug_response_payload: Mutex<Option<serde_json::Value>>, |
There was a problem hiding this comment.
seeing this again is a bit confusing. Since we have all the types defined we could change it to:
request_payload: BridgeRequestPayload
response_payload: Option<Serde::Value>
the response can remain a serde value because we dont know what is coming from the bridge
| _debug = enabled; | ||
| } | ||
|
|
||
| export function setDebugReportHandler( |
There was a problem hiding this comment.
high level. I think this can be simplified even more.
- i would avoid the debug handler unless there is a real need for it. the
getDebugReportmethod should be enough
| return value; | ||
| } | ||
|
|
||
| function cloneValue(value: unknown): unknown { |
| }); | ||
| } | ||
|
|
||
| function normalizeForJson(value: unknown): unknown { |
There was a problem hiding this comment.
do we have bigints in the payload? where exactly are they coming from?
| return report; | ||
| } | ||
|
|
||
| function compact<T extends Record<string, unknown>>(value: T): T { |
| return report; | ||
| } | ||
|
|
||
| export function updateDebugReport( |
There was a problem hiding this comment.
If you wanted to make the debug report stateful and keep changing the status you should use a class and have method .create, .update
i wouldn't track status to begin with, lets make it something that you initialize once
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 41beb9c. Configure here.
| requestPayload: readBridgeDebugPayload(wasmRequest), | ||
| requestId, | ||
| connectorURI, | ||
| }); |
There was a problem hiding this comment.
Bridge debug payload not cached
Medium Severity
Bridge getDebugReport() loads request_payload via live debugPayload() while request_id and connector_uri are fixed at construction. During an in-flight pollOnce(), WASM temporarily clears the connection, debugPayload() fails, and the report can omit request_payload even in debug mode.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 41beb9c. Configure here.


Adds structured debug reporting across the bridge (URL/QR + invite-code), WASM, and
World App native (mini_app) request flows. When IDKit debug mode is enabled, each
request builds a report tracking transport, status, timestamps, request/connector
metadata, the pre-encryption request payload, bridge create/poll response snapshots,
and errors including bridge-creation failures that throw before a request object
exists. Integrators read reports via
getDebugReport()on request objects, or byregistering
setDebugReportHandler()for live lifecycle updates.Note
Medium Risk
Debug reports can include sensitive RP context and verification payloads when enabled; production paths are unchanged when debug is off.
Overview
Adds opt-in debug reporting when
setDebug(true)orwindow.IDKIT_DEBUGis set. Integrators can callgetDebugReport()on bridge (URL/QR and invite-code) and World App native requests to get a structured snapshot: package version, transport (bridge|mini_app), timestamps,request_id,connector_uri, optional request/response payloads, and mini-app metadata (World App version, platform) for native flows only.The JS layer introduces
createIDKitDebugReportand exportsIDKitDebugReport/IDKitDebugTransport. Bridge reports pull the pre-encryption request JSON from WASM via newdebugPayload()on request handles; RustBridgeConnectionnow retains that payload at create time (keys, IVs, and encrypted bodies are explicitly not included). Native transport stores the raw host response forresponse_payloadafter verify completes.Tests cover disabled debug (no report), bridge vs mini_app shape, and native lifecycle snapshots.
Reviewed by Cursor Bugbot for commit 41beb9c. Bugbot is set up for automated code reviews on this repo. Configure here.