fix: address all 13 CodeRabbit findings for Phase 2 CEF code

1. messaging.go: Info→Debug for per-message dispatch logging
2. messaging.go: data race on baseCtx — wrap SetBaseContext/HandleMessage with mutex
3. content_injector.go: escape id parameter in generated JS to prevent injection
4. content_injector.go: add U+2028/U+2029 escaping in escapeForJSString
5. content_injector.go: fix message bridge shim — use MessageBridgeJS constant
   with X-Dumber-Body header (was sending raw body, handler expected header)
6. content_injector.go: fix internalSchemePrefix from cef://dumber to dumb://
7. find_controller.go: remove unused lastID field (dead code)
8. find_controller.go: defensive check for negative count before uint cast
9. engine.go: encapsulate SetColorResolver via contentInjector method
10. engine.go: RegisterHandlers stub now logs Warn instead of misleading Info
11. engine_init.go: check RegisterSchemeHandlerFactory result, log error on failure
12. engine_init.go: document os.Args thread-safety assumption
13. scheme_handler.go: use assetsSet bool instead of embed.FS zero-value comparison,
    HTML-escape error messages, handle json.Marshal errors

commit: 1455aeba2cb83e6fe61d2a7c6a41e6ca4c16beab
author: brice · 7mo ago · 2026-03-21 21:34 UTC
committer: brice · 6mo ago · 2026-03-28 04:38 UTC
parent: 7b06939
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

6 files · +46 -37

M internal/infrastructure/cef/content_injector.go +15 -19


@@ -15,7 +15,7 @@ var _ port.ContentInjector = (*contentInjector)(nil)
15	15
16	16		// internalSchemePrefix is used to detect internal pages that receive dark mode
17	17		// and message bridge scripts. Must match the engine's InternalSchemePath prefix.
18		-	const internalSchemePrefix = "cef://dumber"
	18	+	const internalSchemePrefix = "dumb://"
19	19
20	20		// contentInjector implements port.ContentInjector for the CEF engine.
21	21		// It stores CSS strings and injects them into webviews via ExecuteJavaScript.

@@ -29,6 +29,13 @@ type contentInjector struct {
29	29		colorResolver port.ColorSchemeResolver
30	30		}
31	31
	32	+	// setColorResolver updates the color scheme resolver used for dark mode detection.
	33	+	func (ci *contentInjector) setColorResolver(resolver port.ColorSchemeResolver) {
	34	+	ci.mu.Lock()
	35	+	defer ci.mu.Unlock()
	36	+	ci.colorResolver = resolver
	37	+	}
	38	+
32	39		// newContentInjector creates a content injector wired to the given engine.
33	40		func newContentInjector(engine Engine, resolver port.ColorSchemeResolver) contentInjector {
34	41		return &contentInjector{

@@ -125,12 +132,13 @@ func (ci contentInjector) onLoadEnd(wv WebView) {
125	132
126	133		// injectCSS injects a CSS string as a <style> element via JavaScript.
127	134		func (ci contentInjector) injectCSS(wv WebView, id, css string) {
	135	+	escapedID := escapeForJSString(id)
128	136		escaped := escapeForJSString(css)
129	137		script := fmt.Sprintf(`(function(){
130	138		var el = document.getElementById('%s');
131	139		if (!el) { el = document.createElement('style'); el.id = '%s'; document.head.appendChild(el); }
132	140		el.textContent = '%s';
133		-	})();`, id, id, escaped)
	141	+	})();`, escapedID, escapedID, escaped)
134	142
135	143		wv.RunJavaScript(context.Background(), script)
136	144		}

@@ -191,24 +199,10 @@ func (ci contentInjector) injectDarkModeScript(wv WebView, prefersDark bool) {
191	199
192	200		// injectMessageBridgeShim injects the window.dumber.postMessage JS client shim
193	201		// so internal pages can communicate with Go handlers via fetch.
	202	+	// The message body is base64-encoded into the X-Dumber-Body header to work
	203	+	// around purego-cef's unexported PostData element wrapper.
194	204		func (ci contentInjector) injectMessageBridgeShim(wv WebView) {
195		-	const script = `(function() {
196		-	window.dumber = window.dumber \|\| {};
197		-	window.dumber.postMessage = function(msg) {
198		-	return fetch('/api/message', {
199		-	method: 'POST',
200		-	headers: {'Content-Type': 'application/json'},
201		-	body: JSON.stringify(msg)
202		-	}).then(function(r) { return r.json(); }).then(function(resp) {
203		-	if (resp && resp._callback && window[resp._callback]) {
204		-	window[resp._callback](resp.data);
205		-	}
206		-	return resp;
207		-	});
208		-	};
209		-	})();`
210		-
211		-	wv.RunJavaScript(context.Background(), script)
	205	+	wv.RunJavaScript(context.Background(), MessageBridgeJS)
212	206		}
213	207
214	208		// escapeForJSString escapes a string for use inside a JS single-quoted string literal.

@@ -217,5 +211,7 @@ func escapeForJSString(s string) string {
217	211		s = strings.ReplaceAll(s, "'", "\\'")
218	212		s = strings.ReplaceAll(s, "\n", "\\n")
219	213		s = strings.ReplaceAll(s, "\r", "\\r")
	214	+	s = strings.ReplaceAll(s, "\u2028", "\\u2028")
	215	+	s = strings.ReplaceAll(s, "\u2029", "\\u2029")
220	216		return s
221	217		}

 // internalSchemePrefix is used to detect internal pages that receive dark mode
 // and message bridge scripts. Must match the engine's InternalSchemePath prefix.
-const internalSchemePrefix = "cef://dumber"
+const internalSchemePrefix = "dumb://"
 // contentInjector implements port.ContentInjector for the CEF engine.
 // It stores CSS strings and injects them into webviews via ExecuteJavaScript.
 	colorResolver    port.ColorSchemeResolver
 }
+// setColorResolver updates the color scheme resolver used for dark mode detection.
+func (ci *contentInjector) setColorResolver(resolver port.ColorSchemeResolver) {
+	ci.mu.Lock()
+	defer ci.mu.Unlock()
+	ci.colorResolver = resolver
+}
 // newContentInjector creates a content injector wired to the given engine.
 func newContentInjector(engine *Engine, resolver port.ColorSchemeResolver) *contentInjector {
 	return &contentInjector{
 // injectCSS injects a CSS string as a <style> element via JavaScript.
 func (ci *contentInjector) injectCSS(wv *WebView, id, css string) {
+	escapedID := escapeForJSString(id)
 	escaped := escapeForJSString(css)
 	script := fmt.Sprintf(`(function(){
   var el = document.getElementById('%s');
   if (!el) { el = document.createElement('style'); el.id = '%s'; document.head.appendChild(el); }
   el.textContent = '%s';
-})();`, id, id, escaped)
+})();`, escapedID, escapedID, escaped)
 	wv.RunJavaScript(context.Background(), script)
 }
 // injectMessageBridgeShim injects the window.dumber.postMessage JS client shim
 // so internal pages can communicate with Go handlers via fetch.
+// The message body is base64-encoded into the X-Dumber-Body header to work
+// around purego-cef's unexported PostData element wrapper.
 func (ci *contentInjector) injectMessageBridgeShim(wv *WebView) {
-	const script = `(function() {
+	wv.RunJavaScript(context.Background(), MessageBridgeJS)
-  window.dumber = window.dumber || {};
-  window.dumber.postMessage = function(msg) {
-    return fetch('/api/message', {
-      method: 'POST',
-      headers: {'Content-Type': 'application/json'},
-      body: JSON.stringify(msg)
-    }).then(function(r) { return r.json(); }).then(function(resp) {
-      if (resp && resp._callback && window[resp._callback]) {
-        window[resp._callback](resp.data);
-      }
-      return resp;
-    });
-  };
-})();`
-	wv.RunJavaScript(context.Background(), script)
 }
 // escapeForJSString escapes a string for use inside a JS single-quoted string literal.
 	s = strings.ReplaceAll(s, "'", "\\'")
 	s = strings.ReplaceAll(s, "\n", "\\n")
 	s = strings.ReplaceAll(s, "\r", "\\r")
+	s = strings.ReplaceAll(s, "\u2028", "\\u2028")
+	s = strings.ReplaceAll(s, "\u2029", "\\u2029")
 	return s
 }

M internal/infrastructure/cef/engine.go +2 -6


@@ -97,9 +97,7 @@ func (e *Engine) FaviconDatabase() port.FaviconDatabase {
97	97		// engine creation (e.g., from bootstrap wiring).
98	98		func (e *Engine) SetColorResolver(resolver port.ColorSchemeResolver) {
99	99		if e.contentInj != nil {
100		-	e.contentInj.mu.Lock()
101		-	e.contentInj.colorResolver = resolver
102		-	e.contentInj.mu.Unlock()
	100	+	e.contentInj.setColorResolver(resolver)
103	101		}
104	102		}
105	103

@@ -138,15 +136,13 @@ func (e *Engine) RegisterHandlers(ctx context.Context, deps port.HandlerDependen
138	136		return nil
139	137		}
140	138		log := logging.FromContext(ctx)
141		-	log.Debug().Msg("cef: registering message handlers (deps available, handler wiring pending)")
142		-
143	139		// TODO: Wire homepage, config, keybindings, and clipboard handlers
144	140		// into e.messageRouter. Each handler implements MessageHandler and is
145	141		// registered by message type (e.g. "get_recent_history", "get_favorites").
146	142		// The CEF MessageRouter dispatches based on Message.Type from the JS bridge.
147	143		_ = deps
148	144
149		-	log.Info().Msg("cef: message handlers registered")
	145	+	log.Warn().Msg("cef: RegisterHandlers stub — handler wiring not yet implemented")
150	146		return nil
151	147		}
152	148

 // engine creation (e.g., from bootstrap wiring).
 func (e *Engine) SetColorResolver(resolver port.ColorSchemeResolver) {
 	if e.contentInj != nil {
-		e.contentInj.mu.Lock()
+		e.contentInj.setColorResolver(resolver)
-		e.contentInj.colorResolver = resolver
-		e.contentInj.mu.Unlock()
 	}
 }
 		return nil
 	}
 	log := logging.FromContext(ctx)
-	log.Debug().Msg("cef: registering message handlers (deps available, handler wiring pending)")
 	// TODO: Wire homepage, config, keybindings, and clipboard handlers
 	// into e.messageRouter. Each handler implements MessageHandler and is
 	// registered by message type (e.g. "get_recent_history", "get_favorites").
 	// The CEF MessageRouter dispatches based on Message.Type from the JS bridge.
 	_ = deps
-	log.Info().Msg("cef: message handlers registered")
+	log.Warn().Msg("cef: RegisterHandlers stub — handler wiring not yet implemented")
 	return nil
 }

M internal/infrastructure/cef/engine_init.go +6 -1


@@ -33,6 +33,7 @@ func NewEngine(ctx context.Context, cfg config.CEFEngineConfig) (*Engine, error)
33	33		settings := prepareCEFSettings(cfg, logger)
34	34
35	35		// Inject --no-zygote temporarily for cef_initialize, then restore.
	36	+	// Safe: runs during single-threaded startup before concurrent goroutines.
36	37		savedArgs := os.Args
37	38		os.Args = appendIfMissing(os.Args, "--no-zygote")
38	39

@@ -144,7 +145,11 @@ func wireEngine(
144	145		schemeHandler.setAssets(assets.WebUIAssets)
145	146
146	147		result := purecef.RegisterSchemeHandlerFactory("dumb", "", purecef.NewSchemeHandlerFactory(schemeHandler))
147		-	logger.Info().Int32("result", result).Msg("cef: registered dumb:// scheme handler factory")
	148	+	if result != 1 {
	149	+	logger.Error().Int32("result", result).Msg("cef: failed to register dumb:// scheme handler factory")
	150	+	} else {
	151	+	logger.Info().Msg("cef: registered dumb:// scheme handler factory")
	152	+	}
148	153
149	154		eng.messageRouter = messageRouter
150	155		eng.schemeHandler = schemeHandler

 	settings := prepareCEFSettings(cfg, logger)
 	// Inject --no-zygote temporarily for cef_initialize, then restore.
+	// Safe: runs during single-threaded startup before concurrent goroutines.
 	savedArgs := os.Args
 	os.Args = appendIfMissing(os.Args, "--no-zygote")
 	schemeHandler.setAssets(assets.WebUIAssets)
 	result := purecef.RegisterSchemeHandlerFactory("dumb", "", purecef.NewSchemeHandlerFactory(schemeHandler))
-	logger.Info().Int32("result", result).Msg("cef: registered dumb:// scheme handler factory")
+	if result != 1 {
+		logger.Error().Int32("result", result).Msg("cef: failed to register dumb:// scheme handler factory")
+	} else {
+		logger.Info().Msg("cef: registered dumb:// scheme handler factory")
+	}
 	eng.messageRouter = messageRouter
 	eng.schemeHandler = schemeHandler

M internal/infrastructure/cef/find_controller.go +6 -5


@@ -16,7 +16,6 @@ type cefFindController struct {
16	16		mu sync.RWMutex
17	17		host purecef.BrowserHost
18	18		searchTxt string
19		-	lastID int32 // incremental identifier for the current search
20	19		matchCase int32 // cached from most recent Search call
21	20
22	21		// counting tracks whether the current search is a CountMatches request

@@ -48,7 +47,6 @@ func (fc *cefFindController) setHost(host purecef.BrowserHost) {
48	47		func (fc *cefFindController) Search(text string, opts port.FindOptions, _ uint) {
49	48		fc.mu.Lock()
50	49		fc.searchTxt = text
51		-	fc.lastID++
52	50		fc.counting = false
53	51		mc := matchCaseFromOpts(opts)
54	52		fc.matchCase = mc

@@ -65,7 +63,6 @@ func (fc *cefFindController) Search(text string, opts port.FindOptions, _ uint)
65	63		func (fc *cefFindController) CountMatches(text string, opts port.FindOptions, _ uint) {
66	64		fc.mu.Lock()
67	65		fc.searchTxt = text
68		-	fc.lastID++
69	66		fc.counting = true
70	67		mc := matchCaseFromOpts(opts)
71	68		fc.matchCase = mc

@@ -206,14 +203,18 @@ func (fc *cefFindController) handleFindResult(_, count, _, finalUpdate int32) {
206	203		}
207	204		fc.mu.RUnlock()
208	205
	206	+	var safeCount uint
	207	+	if count > 0 {
	208	+	safeCount = uint(count)
	209	+	}
209	210		for _, cb := range foundCBs {
210		-	cb(uint(count))
	211	+	cb(safeCount)
211	212		}
212	213		for _, cb := range failedCBs {
213	214		cb()
214	215		}
215	216		for _, cb := range countedCBs {
216		-	cb(uint(count))
	217	+	cb(safeCount)
217	218		}
218	219		}
219	220

 	mu        sync.RWMutex
 	host      purecef.BrowserHost
 	searchTxt string
-	lastID    int32 // incremental identifier for the current search
 	matchCase int32 // cached from most recent Search call
 	// counting tracks whether the current search is a CountMatches request
 func (fc *cefFindController) Search(text string, opts port.FindOptions, _ uint) {
 	fc.mu.Lock()
 	fc.searchTxt = text
-	fc.lastID++
 	fc.counting = false
 	mc := matchCaseFromOpts(opts)
 	fc.matchCase = mc
 func (fc *cefFindController) CountMatches(text string, opts port.FindOptions, _ uint) {
 	fc.mu.Lock()
 	fc.searchTxt = text
-	fc.lastID++
 	fc.counting = true
 	mc := matchCaseFromOpts(opts)
 	fc.matchCase = mc
 	}
 	fc.mu.RUnlock()
+	var safeCount uint
+	if count > 0 {
+		safeCount = uint(count)
+	}
 	for _, cb := range foundCBs {
-		cb(uint(count))
+		cb(safeCount)
 	}
 	for _, cb := range failedCBs {
 		cb()
 	}
 	for _, cb := range countedCBs {
-		cb(uint(count))
+		cb(safeCount)
 	}
 }

M internal/infrastructure/cef/messaging.go +8 -3


@@ -64,7 +64,9 @@ func (r *MessageRouter) SetBaseContext(ctx context.Context) {
64	64		if ctx == nil {
65	65		ctx = context.Background()
66	66		}
	67	+	r.mu.Lock()
67	68		r.baseCtx = ctx
	69	+	r.mu.Unlock()
68	70		}
69	71
70	72		// RegisterHandler registers a handler for a message type.

@@ -108,7 +110,10 @@ func (r *MessageRouter) RegisterHandlerWithCallbacks(msgType, callback, errorCal
108	110		// HandleMessage decodes a raw JSON message body and routes it to the correct handler.
109	111		// Returns the JSON-encoded response. Called by the scheme handler for /api/message POST.
110	112		func (r *MessageRouter) HandleMessage(ctx context.Context, webviewID uint64, body []byte) ([]byte, error) {
111		-	log := logging.FromContext(r.baseCtx).With().Str("component", "message-router").Logger()
	113	+	r.mu.RLock()
	114	+	baseCtx := r.baseCtx
	115	+	r.mu.RUnlock()
	116	+	log := logging.FromContext(baseCtx).With().Str("component", "message-router").Logger()
112	117
113	118		var msg Message
114	119		if err := json.Unmarshal(body, &msg); err != nil {

@@ -133,14 +138,14 @@ func (r *MessageRouter) HandleMessage(ctx context.Context, webviewID uint64, bod
133	138		return json.Marshal(map[string]string{"error": "unknown message type: " + msg.Type})
134	139		}
135	140
136		-	log.Info().
	141	+	log.Debug().
137	142		Str("type", msg.Type).
138	143		Uint64("webview_id", msg.WebViewID).
139	144		Int("payload_len", len(msg.Payload)).
140	145		Msg("dispatching message to handler")
141	146
142	147		if ctx == nil {
143		-	ctx = r.baseCtx
	148	+	ctx = baseCtx
144	149		}
145	150		resp, err := entry.handler.Handle(ctx, msg.WebViewID, msg.Payload)
146	151		if err != nil {

 	if ctx == nil {
 		ctx = context.Background()
 	}
+	r.mu.Lock()
 	r.baseCtx = ctx
+	r.mu.Unlock()
 }
 // RegisterHandler registers a handler for a message type.
 // HandleMessage decodes a raw JSON message body and routes it to the correct handler.
 // Returns the JSON-encoded response. Called by the scheme handler for /api/message POST.
 func (r *MessageRouter) HandleMessage(ctx context.Context, webviewID uint64, body []byte) ([]byte, error) {
-	log := logging.FromContext(r.baseCtx).With().Str("component", "message-router").Logger()
+	r.mu.RLock()
+	baseCtx := r.baseCtx
+	r.mu.RUnlock()
+	log := logging.FromContext(baseCtx).With().Str("component", "message-router").Logger()
 	var msg Message
 	if err := json.Unmarshal(body, &msg); err != nil {
 		return json.Marshal(map[string]string{"error": "unknown message type: " + msg.Type})
 	}
-	log.Info().
+	log.Debug().
 		Str("type", msg.Type).
 		Uint64("webview_id", msg.WebViewID).
 		Int("payload_len", len(msg.Payload)).
 		Msg("dispatching message to handler")
 	if ctx == nil {
-		ctx = r.baseCtx
+		ctx = baseCtx
 	}
 	resp, err := entry.handler.Handle(ctx, msg.WebViewID, msg.Payload)
 	if err != nil {

M internal/infrastructure/cef/scheme_handler.go +9 -3


@@ -36,6 +36,7 @@ type dumbSchemeHandler struct {
36	36		ctx context.Context
37	37		messageRouter *MessageRouter
38	38		assets embed.FS
	39	+	assetsSet bool
39	40		assetDir string
40	41		logger zerolog.Logger
41	42		mu sync.RWMutex

@@ -57,6 +58,7 @@ func (h *dumbSchemeHandler) setAssets(assets embed.FS) {
57	58		h.mu.Lock()
58	59		defer h.mu.Unlock()
59	60		h.assets = assets
	61	+	h.assetsSet = true
60	62		h.logger.Debug().Msg("assets filesystem configured")
61	63		}
62	64

@@ -129,7 +131,7 @@ func (h dumbSchemeHandler) handleConfigAPI(cfg config.Config) purecef.Resource
129	131		// handleAsset serves static files from the embedded filesystem.
130	132		func (h dumbSchemeHandler) handleAsset(u url.URL) purecef.ResourceHandler {
131	133		h.mu.RLock()
132		-	hasAssets := h.assets != (embed.FS{})
	134	+	hasAssets := h.assetsSet
133	135		assetDir := h.assetDir
134	136		h.mu.RUnlock()
135	137

@@ -265,12 +267,16 @@ func (h *dumbSchemeHandler) newRawResourceHandler(status int, contentType string
265	267		}
266	268
267	269		func (h *dumbSchemeHandler) newErrorResourceHandler(status int, msg string) purecef.ResourceHandler {
268		-	body := fmt.Sprintf(`<!DOCTYPE html><html><body><h1>%d</h1><p>%s</p></body></html>`, status, msg)
	270	+	escaped := strings.ReplaceAll(strings.ReplaceAll(msg, "&", "&"), "<", "<")
	271	+	body := fmt.Sprintf(`<!DOCTYPE html><html><body><h1>%d</h1><p>%s</p></body></html>`, status, escaped)
269	272		return h.newRawResourceHandler(status, "text/html; charset=utf-8", []byte(body))
270	273		}
271	274
272	275		func (h *dumbSchemeHandler) newJSONResourceHandler(status int, v any) purecef.ResourceHandler {
273		-	data, _ := json.Marshal(v)
	276	+	data, err := json.Marshal(v)
	277	+	if err != nil {
	278	+	return h.newErrorResourceHandler(http.StatusInternalServerError, "JSON encoding failed")
	279	+	}
274	280		return h.newRawResourceHandler(status, "application/json", data)
275	281		}
276	282

 	ctx           context.Context
 	messageRouter *MessageRouter
 	assets        embed.FS
+	assetsSet     bool
 	assetDir      string
 	logger        zerolog.Logger
 	mu            sync.RWMutex
 	h.mu.Lock()
 	defer h.mu.Unlock()
 	h.assets = assets
+	h.assetsSet = true
 	h.logger.Debug().Msg("assets filesystem configured")
 }
 // handleAsset serves static files from the embedded filesystem.
 func (h *dumbSchemeHandler) handleAsset(u *url.URL) purecef.ResourceHandler {
 	h.mu.RLock()
-	hasAssets := h.assets != (embed.FS{})
+	hasAssets := h.assetsSet
 	assetDir := h.assetDir
 	h.mu.RUnlock()
 }
 func (h *dumbSchemeHandler) newErrorResourceHandler(status int, msg string) purecef.ResourceHandler {
-	body := fmt.Sprintf(`<!DOCTYPE html><html><body><h1>%d</h1><p>%s</p></body></html>`, status, msg)
+	escaped := strings.ReplaceAll(strings.ReplaceAll(msg, "&", "&amp;"), "<", "&lt;")
+	body := fmt.Sprintf(`<!DOCTYPE html><html><body><h1>%d</h1><p>%s</p></body></html>`, status, escaped)
 	return h.newRawResourceHandler(status, "text/html; charset=utf-8", []byte(body))
 }
 func (h *dumbSchemeHandler) newJSONResourceHandler(status int, v any) purecef.ResourceHandler {
-	data, _ := json.Marshal(v)
+	data, err := json.Marshal(v)
+	if err != nil {
+		return h.newErrorResourceHandler(http.StatusInternalServerError, "JSON encoding failed")
+	}
 	return h.newRawResourceHandler(status, "application/json", data)
 }