Repository navigation
Conversation
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, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 501c80c. Configure here.
| if event.Message.SessionID != "" { | ||
| h.protocolEvent(event.Message.SessionID, event.Message.Method, event.Message.Params) | ||
| } | ||
| } |
There was a problem hiding this comment.
Failed attaches never reconnect
Medium Severity
handleEvent ignores EventDiscoveryFailed. A failed Target.attachToTarget therefore never tears down the hook connection, so that page or iframe stays unhooked until some later socket loss. cdpmonitor cancels the connection on this event so attach is retried; the new hook loop does not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 501c80c. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
This adds a WebAuthn page hook. When it is on, every page and iframe document answers
navigator.credentials.get()/create()publicKey requests through the VM instead of the browser's authenticators. kernel-images forwards each request to metro-api along with the calling document's origin as Chromium reports it. Private keys never enter the VM. Anything metro-api doesn't answer falls back to the browser's own implementation.PUT /webauthn/hook(new,x-telemetry-category: platform){"enabled": bool}, response 200{"enabled": bool}. It is idempotent./run/kernel/webauthn-hook.jsonbefore it takes effect, the same way the egress policy is, so a restart of the API process restores it. If the flag can't be persisted, the request returns 500.Page.addScriptToEvaluateOnNewDocumentwithrunImmediately, and the script guards itself so it never installs twice.not_handledwithout contacting metro-api.Runtime.enable,Page.enable,Runtime.addBinding, or the script registration), it is retried once after 1 s and logged at Error if it still fails.Its own DevTools connection
The hook does not share telemetry's connection;
server/lib/cdpmonitoris unchanged. CDP domain state,Runtime.addBindingbindings andPage.addScriptToEvaluateOnNewDocumentscripts are per session, and sessions belong to a connection, so telemetry turning capture on and off does not affect the hook.Following
webmcpclient's pattern, the hook dialscdpclient.DialWithEventson the upstream manager's current DevTools URL and uses abrowsersurfacetracker for page and iframe targets (no target pausing; decision 11 stands). Unlike WebMCP's lazy connection, the connection exists only while the hook is on:Page protocol (
lib/webauthnhook/hook.js)CredentialsContainer.prototype.get/createonly forpublicKeyrequests. Password, federated and OTP requests go straight to the browser.isUserVerifyingPlatformAuthenticatorAvailable()andisConditionalMediationAvailable()report true, and passkey capabilities are merged intogetClientCapabilities().signalis honored: an abort rejects withAbortError.{id, op: "abort"}. kernel-images drops the call if it hasn't reached metro-api yet, cancels the in-flight HTTP request and its retries if it has, and sends no answer.{suspend: true}(on disable) and the script sends itself{resume: true}when it runs again in the same document (on re-enable).instanceof PublicKeyCredential/AuthenticatorAttestationResponse/AuthenticatorAssertionResponseand[object PublicKeyCredential]hold. They expose ArrayBuffer fields,toJSON(),getClientExtensionResults(),getAuthenticatorData(),getPublicKey(),getPublicKeyAlgorithm()andgetTransports(), withauthenticatorAttachment: "platform".Origin handling
Runtime.executionContextCreatedfor the binding call'sexecutionContextId, main world only.parentFrameId.localhostand*.localhost. Opaque and other insecure origins getnot_handled.Page.getPermissionsPolicyState) must allowpublickey-credentials-get/-create. If it doesn't, the request is left to the browser, which rejects it exactly as it would without the hook.metro-api client (
lib/webauthnhook/signer.go)POST {KERNEL_METRO_API_BASE_URL}/webauthn/{get|create}withAuthorization: Bearer <KERNEL_INSTANCE_JWT>. The full HTTP contract is documented onMetroSignerinsigner.go.{origin, top_origin, mediation?, public_key}.origin,top_origin,mediation,public_key), so metro-api can replay a stored answer for a retriedcreate. Any other non-200 status (including 401/403), a malformed body, or a credential missing required fields leaves the request to the browser. An unknown error name becomesNotAllowedError.instanceIdentityProvider(renamed fromotlp_credential.go), which also yields the metro-api URL from the applied fork-identity payload (forkidentity.MetroAPIURL, preferringkernel_metro_api_base_url).Testing
go vet ./...: passes.go test -race $(go list ./... | grep -v /e2e$): passes.make test-runtime: passes.KERNEL_CDPMONITOR_CHROME_E2E=1 go test -race ./lib/cdpmonitor -count=1: passes (cdpmonitor itself is unchanged).KERNEL_WEBAUTHN_CHROME_E2E=1 go test -race ./lib/webauthnhook -count=1: passes, 3 consecutive runs, against Chromium 154. A telemetry monitor runs alongside on its own connection. It uses HTTPS sites under*.testand a fake metro-api that registers an ES256 key and signs real assertions. It covers:create()returns an attestation that parses (fmtnone, flags UP|UV|BE|BS|AT, counter 0, zero AAGUID, COSE key matchinggetPublicKey()).get()returns an assertion whose signature verifies against the registered key.get()issued at page load is answered.get()at load is not answered on its first document; a reload is (pins decision 11).not_handledfalls back to the native implementation (virtual authenticator); an error answer rejects with thatDOMException.allowis left to the browser.instanceofandtoJSON()matches every buffer; password credential requests are untouched; an aborted signal rejects withAbortError.Unit tests additionally cover the unanswered-call cap, oldest-first eviction of answered calls, abort before and after dispatch, top-origin selection during navigation, the setup retry, identical retry bodies, starting and stopping the connection with the flag, and restoring the flag across restarts.
Not run:
make test-e2e(Docker image e2e) and any built-image or staging check. Standby/wake and snapshot-fork behavior of the hook are not verified.Notes for the metro-api side
createretries: kernel-images retriescreateon 5xx and transport errors with an identical body, relying on metro-api replaying the stored answer for a retried create.credentialanswer must includeid,rawId,response.clientDataJSONandresponse.authenticatorData, plusresponse.signatureforgetorresponse.attestationObjectforcreate. Otherwise kernel-images treats it asnot_handled. Includeresponse.publicKey,response.publicKeyAlgorithmandresponse.transportson registrations sogetPublicKey()and related methods return values.public_keyis page-supplied: treat it as untrusted. A page can call the binding directly.originandtop_originare always Chromium's.mediationis forwarded forcreatetoo: for example"conditional"for conditional create.Known gaps
window.open,target=_blank, sandboxed iframes) need a reload. Targets are not paused on attach; a test pins this behavior.