fix(webview): harden reviewed bridge paths (#372)
- commit
- 1ee1a70f5f47523cf6bc8b19905cfe58cbf455d3
- author
- bnema · · 2026-08-13 08:14 UTC
- committer
- GitHub · · 2026-08-13 08:14 UTC
- parent
- f477efa
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
3 files · +8 -5
M internal/infrastructure/webkit/messaging.go +4 -3
| @@ -282,7 +282,7 @@ func decodeScriptMessage(valuePtr uintptr, log zerolog.Logger) (Message, bool) { | |||
| 282 | 282 | ||
| 283 | 283 | var msg Message | |
| 284 | 284 | if err := json.Unmarshal([]byte(rawJSON), &msg); err != nil { | |
| 285 | - | log.Warn().Err(err).Str("json", rawJSON).Msg("failed to unmarshal script message") | |
| 285 | + | log.Warn().Err(err).Int("json_len", len(rawJSON)).Msg("failed to unmarshal script message") | |
| 286 | 286 | return Message{}, false | |
| 287 | 287 | } | |
| 288 | 288 | return msg, true | |
| @@ -305,14 +305,15 @@ func (r *MessageRouter) handleAllowlistedBridgeMessage(senderWV *WebView, msg Me | |||
| 305 | 305 | Editable bool `json:"editable"` | |
| 306 | 306 | Token string `json:"token"` | |
| 307 | 307 | } | |
| 308 | + | log := logging.FromContext(r.baseContext()) | |
| 308 | 309 | if len(msg.Payload) != 0 { | |
| 309 | 310 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | |
| 310 | - | logging.FromContext(r.baseCtx).Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | |
| 311 | + | log.Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | |
| 311 | 312 | return true | |
| 312 | 313 | } | |
| 313 | 314 | } | |
| 314 | 315 | if payload.Token == "" || !senderWV.matchesEditableFocusBridgeToken(payload.Token) { | |
| 315 | - | logging.FromContext(r.baseCtx).Warn().Str("type", msg.Type).Msg("rejecting allowlisted bridge message with invalid token") | |
| 316 | + | log.Warn().Str("type", msg.Type).Msg("rejecting allowlisted bridge message with invalid token") | |
| 316 | 317 | return true | |
| 317 | 318 | } | |
| 318 | 319 | senderWV.dispatchEditableFocusChanged(payload.Editable) | |
| @@ -282,7 +282,7 @@ func decodeScriptMessage(valuePtr uintptr, log zerolog.Logger) (Message, bool) { | |||||
| 282 | 282 | ||||
| 283 | var msg Message | 283 | var msg Message | ||
| 284 | if err := json.Unmarshal([]byte(rawJSON), &msg); err != nil { | 284 | if err := json.Unmarshal([]byte(rawJSON), &msg); err != nil { | ||
| 285 | - | log.Warn().Err(err).Str("json", rawJSON).Msg("failed to unmarshal script message") | 285 | + | log.Warn().Err(err).Int("json_len", len(rawJSON)).Msg("failed to unmarshal script message") |
| 286 | return Message{}, false | 286 | return Message{}, false | ||
| 287 | } | 287 | } | ||
| 288 | return msg, true | 288 | return msg, true | ||
| @@ -305,14 +305,15 @@ func (r *MessageRouter) handleAllowlistedBridgeMessage(senderWV *WebView, msg Me | |||||
| 305 | Editable bool `json:"editable"` | 305 | Editable bool `json:"editable"` | ||
| 306 | Token string `json:"token"` | 306 | Token string `json:"token"` | ||
| 307 | } | 307 | } | ||
| 308 | + | log := logging.FromContext(r.baseContext()) | |||
| 308 | if len(msg.Payload) != 0 { | 309 | if len(msg.Payload) != 0 { | ||
| 309 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | 310 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | ||
| 310 | - | logging.FromContext(r.baseCtx).Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | 311 | + | log.Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") |
| 311 | return true | 312 | return true | ||
| 312 | } | 313 | } | ||
| 313 | } | 314 | } | ||
| 314 | if payload.Token == "" || !senderWV.matchesEditableFocusBridgeToken(payload.Token) { | 315 | if payload.Token == "" || !senderWV.matchesEditableFocusBridgeToken(payload.Token) { | ||
| 315 | - | logging.FromContext(r.baseCtx).Warn().Str("type", msg.Type).Msg("rejecting allowlisted bridge message with invalid token") | 316 | + | log.Warn().Str("type", msg.Type).Msg("rejecting allowlisted bridge message with invalid token") |
| 316 | return true | 317 | return true | ||
| 317 | } | 318 | } | ||
| 318 | senderWV.dispatchEditableFocusChanged(payload.Editable) | 319 | senderWV.dispatchEditableFocusChanged(payload.Editable) | ||
M internal/infrastructure/webkit/pool.go +3 -1
| @@ -179,7 +179,9 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 179 | 179 | ||
| 180 | 180 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | |
| 181 | 181 | alreadyAttached := wv.FrontendAttached() | |
| 182 | - | _ = wv.AttachFrontend(ctx, p.injector, p.router) | |
| 182 | + | if err := wv.AttachFrontend(ctx, p.injector, p.router); err != nil { | |
| 183 | + | log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to pooled webview") | |
| 184 | + | } | |
| 183 | 185 | // Reinjection is only for already-attached reuse after ResetForPoolReuse. | |
| 184 | 186 | // First AttachFrontend already injected; avoid an immediate double inject. | |
| 185 | 187 | if needsFrontendReinjectionOnAcquire(alreadyAttached) && p.injector != nil && wv.ucm != nil { | |
| @@ -179,7 +179,9 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 179 | 179 | ||||
| 180 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | 180 | // Ensure frontend is attached even if this WebView was pooled before injection was configured. | ||
| 181 | alreadyAttached := wv.FrontendAttached() | 181 | alreadyAttached := wv.FrontendAttached() | ||
| 182 | - | _ = wv.AttachFrontend(ctx, p.injector, p.router) | 182 | + | if err := wv.AttachFrontend(ctx, p.injector, p.router); err != nil { |
| 183 | + | log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to pooled webview") | |||
| 184 | + | } | |||
| 183 | // Reinjection is only for already-attached reuse after ResetForPoolReuse. | 185 | // Reinjection is only for already-attached reuse after ResetForPoolReuse. | ||
| 184 | // First AttachFrontend already injected; avoid an immediate double inject. | 186 | // First AttachFrontend already injected; avoid an immediate double inject. | ||
| 185 | if needsFrontendReinjectionOnAcquire(alreadyAttached) && p.injector != nil && wv.ucm != nil { | 187 | if needsFrontendReinjectionOnAcquire(alreadyAttached) && p.injector != nil && wv.ucm != nil { | ||
M internal/ui/app_browser_launch_test.go +1 -1
| @@ -1009,7 +1009,7 @@ func TestTestShellToasterCleanupCancelsAutoDismiss(t *testing.T) { | |||
| 1009 | 1009 | toaster.CancelAutoDismiss() | |
| 1010 | 1010 | ||
| 1011 | 1011 | mainContext := glib.MainContextDefault() | |
| 1012 | - | deadline := time.Now().Add(component.ToastBriefDurationMs + 100*time.Millisecond) | |
| 1012 | + | deadline := time.Now().Add(component.ToastBriefDurationMs + 600*time.Millisecond) | |
| 1013 | 1013 | for time.Now().Before(deadline) { | |
| 1014 | 1014 | for mainContext.Pending() { | |
| 1015 | 1015 | mainContext.Iteration(false) | |
| @@ -1009,7 +1009,7 @@ func TestTestShellToasterCleanupCancelsAutoDismiss(t *testing.T) { | |||||
| 1009 | toaster.CancelAutoDismiss() | 1009 | toaster.CancelAutoDismiss() | ||
| 1010 | 1010 | ||||
| 1011 | mainContext := glib.MainContextDefault() | 1011 | mainContext := glib.MainContextDefault() | ||
| 1012 | - | deadline := time.Now().Add(component.ToastBriefDurationMs + 100*time.Millisecond) | 1012 | + | deadline := time.Now().Add(component.ToastBriefDurationMs + 600*time.Millisecond) |
| 1013 | for time.Now().Before(deadline) { | 1013 | for time.Now().Before(deadline) { | ||
| 1014 | for mainContext.Pending() { | 1014 | for mainContext.Pending() { | ||
| 1015 | mainContext.Iteration(false) | 1015 | mainContext.Iteration(false) | ||