Skip to content

Commit a986d2e

Browse files
Fix: 根據最新 PR Review (Qodo, CodeRabbit, Copilot) 進行進一步修正
我針對 PR#863 的最新一輪 review 意見進行了修正,主要包括: 1. **CI Lint `chrome` 未定義問題:** * 我再次確認並標準化 `sidePanel` API 的使用,優先並主要依賴 `Browser.sidePanel`,以解決 CI 環境中的 `chrome is not defined` 問題。 2. **Qodo Merge Pro 高優先級建議:** * **`onBeforeSendHeaders` null 引用:** 我在修改 `headers[i].value` 前,增加了對 `headers[i]` 的存在性檢查。 * **`_proxyOnDisconnect` 競爭條件:** 我改進了監聽器移除邏輯,在移除自身監聽器前,先保存 `port.proxy` 的引用。 * **增強 `SENSITIVE_KEYWORDS`:** 我在敏感詞列表中加入了 `'auth'`, `'key'`, `'credential'`。 * **`redactSensitiveFields` 循環引用:** 我為該函數增加了 `WeakSet` 來檢測和處理循環引用。 * **`manageChatGptTabState` null 檢查:** 我在 `setTimeout` 回調中對 `input.value` 賦值前,增加了對 `input` 的 null 檢查。 * **`prepareForJumpBackNotification` 警告:** 我在 `setInterval` 回調中,如果 `promiseSettled` 已為 true,則添加警告日誌。 3. **`sidePanel` API 跨瀏覽器相容性:** * 我在 `tabs.onUpdated` 監聽器中,實現了更穩健的 `sidePanel.setOptions` 調用邏輯:優先使用 `Browser.sidePanel`,若失敗或不可用,則嘗試 `chrome.sidePanel` (並進行保護性檢查),均失敗則記錄警告。 所有更改均已通過本地 `npm run lint` 檢查。這些修正旨在進一步提升擴充功能的穩定性、安全性、相容性和程式碼品質。
1 parent 789d8fe commit a986d2e

File tree

2 files changed

+116
-98
lines changed

2 files changed

+116
-98
lines changed

src/background/index.mjs

Lines changed: 89 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -58,28 +58,31 @@ const RECONNECT_CONFIG = {
5858
};
5959

6060
const SENSITIVE_KEYWORDS = [
61-
'apikey', // Covers apiKey, customApiKey, claudeApiKey, etc.
62-
'token', // Covers accessToken, refreshToken, etc.
61+
'apikey',
62+
'token',
6363
'secret',
6464
'password',
6565
'kimimoonshotrefreshtoken',
66+
'auth',
67+
'key',
68+
'credential',
6669
];
6770

68-
function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5) {
71+
function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5, seen = new WeakSet()) {
6972
if (recursionDepth > maxDepth) {
70-
// Prevent infinite recursion on circular objects or excessively deep structures
7173
return 'REDACTED_TOO_DEEP';
7274
}
73-
// Handle null, primitives, and functions directly
7475
if (obj === null || typeof obj !== 'object') {
7576
return obj;
7677
}
7778

78-
// Create a new object or array to store redacted fields, ensuring original is not modified
79-
const redactedObj = Array.isArray(obj) ? [] : {};
79+
if (seen.has(obj)) {
80+
return 'REDACTED_CIRCULAR_REFERENCE';
81+
}
82+
seen.add(obj);
8083

84+
const redactedObj = Array.isArray(obj) ? [] : {};
8185
for (const key in obj) {
82-
// Ensure we're only processing own properties
8386
if (Object.prototype.hasOwnProperty.call(obj, key)) {
8487
const lowerKey = key.toLowerCase();
8588
let isSensitive = false;
@@ -89,14 +92,11 @@ function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5) {
8992
break;
9093
}
9194
}
92-
9395
if (isSensitive) {
9496
redactedObj[key] = 'REDACTED';
9597
} else if (typeof obj[key] === 'object') {
96-
// If the value is an object (or array), recurse
97-
redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth);
98+
redactedObj[key] = redactSensitiveFields(obj[key], recursionDepth + 1, maxDepth, seen);
9899
} else {
99-
// Otherwise, copy the value as is
100100
redactedObj[key] = obj[key];
101101
}
102102
}
@@ -108,7 +108,6 @@ function setPortProxy(port, proxyTabId) {
108108
try {
109109
console.debug(`[background] Attempting to connect to proxy tab: ${proxyTabId}`)
110110

111-
// Ensure old listeners on port.proxy are removed if it exists
112111
if (port.proxy) {
113112
try {
114113
if (port._proxyOnMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
@@ -117,14 +116,12 @@ function setPortProxy(port, proxyTabId) {
117116
console.warn('[background] Error removing old listeners from previous port.proxy instance:', e);
118117
}
119118
}
120-
// Also remove listeners from the main port object that this function might have added in a previous call for this port instance
121119
if (port._portOnMessage) port.onMessage.removeListener(port._portOnMessage);
122120
if (port._portOnDisconnect) port.onDisconnect.removeListener(port._portOnDisconnect);
123121

124-
125122
port.proxy = Browser.tabs.connect(proxyTabId, { name: 'background-to-content-script-proxy' })
126123
console.debug(`[background] Successfully connected to proxy tab: ${proxyTabId}`)
127-
port._reconnectAttempts = 0 // Reset retry count on successful connection
124+
port._reconnectAttempts = 0
128125

129126
port._proxyOnMessage = (msg) => {
130127
console.debug('[background] Message from proxy tab:', msg)
@@ -142,18 +139,19 @@ function setPortProxy(port, proxyTabId) {
142139
port._proxyOnDisconnect = () => {
143140
console.warn(`[background] Proxy tab ${proxyTabId} disconnected.`)
144141

145-
// Cleanup this specific proxy's listeners before setting port.proxy to null
146-
if (port.proxy) { // Check if port.proxy is still valid
142+
const proxyRef = port.proxy;
143+
port.proxy = null
144+
145+
if (proxyRef) {
147146
if (port._proxyOnMessage) {
148-
try { port.proxy.onMessage.removeListener(port._proxyOnMessage); }
149-
catch(e) { console.warn("[background] Error removing _proxyOnMessage from disconnected port.proxy:", e); }
147+
try { proxyRef.onMessage.removeListener(port._proxyOnMessage); }
148+
catch(e) { console.warn("[background] Error removing _proxyOnMessage from disconnected proxyRef:", e); }
150149
}
151-
if (port._proxyOnDisconnect) { // port._proxyOnDisconnect is this function itself
152-
try { port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect); }
153-
catch(e) { console.warn("[background] Error removing _proxyOnDisconnect from disconnected port.proxy:", e); }
150+
if (port._proxyOnDisconnect) {
151+
try { proxyRef.onDisconnect.removeListener(port._proxyOnDisconnect); }
152+
catch(e) { console.warn("[background] Error removing _proxyOnDisconnect from disconnected proxyRef:", e); }
154153
}
155154
}
156-
port.proxy = null // Clear the old proxy
157155

158156
port._reconnectAttempts = (port._reconnectAttempts || 0) + 1;
159157
if (port._reconnectAttempts > RECONNECT_CONFIG.MAX_ATTEMPTS) {
@@ -162,7 +160,6 @@ function setPortProxy(port, proxyTabId) {
162160
try { port.onMessage.removeListener(port._portOnMessage); }
163161
catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
164162
}
165-
// Note: _portOnDisconnect on the main port should remain to handle its eventual disconnection.
166163
return;
167164
}
168165

@@ -171,7 +168,7 @@ function setPortProxy(port, proxyTabId) {
171168

172169
setTimeout(() => {
173170
console.debug(`[background] Retrying connection to tab ${proxyTabId}, attempt ${port._reconnectAttempts}.`);
174-
setPortProxy(port, proxyTabId); // Reconnect
171+
setPortProxy(port, proxyTabId);
175172
}, delay);
176173
}
177174

@@ -181,23 +178,24 @@ function setPortProxy(port, proxyTabId) {
181178
try { port.onMessage.removeListener(port._portOnMessage); }
182179
catch(e) { console.warn("[background] Error removing _portOnMessage on main port disconnect:", e); }
183180
}
184-
if (port.proxy) {
181+
const proxyRef = port.proxy;
182+
if (proxyRef) {
185183
if (port._proxyOnMessage) {
186-
try { port.proxy.onMessage.removeListener(port._proxyOnMessage); }
187-
catch(e) { console.warn("[background] Error removing _proxyOnMessage from port.proxy on main port disconnect:", e); }
184+
try { proxyRef.onMessage.removeListener(port._proxyOnMessage); }
185+
catch(e) { console.warn("[background] Error removing _proxyOnMessage from proxyRef on main port disconnect:", e); }
188186
}
189187
if (port._proxyOnDisconnect) {
190-
try { port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect); }
191-
catch(e) { console.warn("[background] Error removing _proxyOnDisconnect from port.proxy on main port disconnect:", e); }
188+
try { proxyRef.onDisconnect.removeListener(port._proxyOnDisconnect); }
189+
catch(e) { console.warn("[background] Error removing _proxyOnDisconnect from proxyRef on main port disconnect:", e); }
192190
}
193191
try {
194-
port.proxy.disconnect();
192+
proxyRef.disconnect();
195193
} catch(e) {
196-
console.warn('[background] Error disconnecting port.proxy on main port disconnect:', e);
194+
console.warn('[background] Error disconnecting proxyRef on main port disconnect:', e);
197195
}
198196
port.proxy = null;
199197
}
200-
if (port._portOnDisconnect) { // Remove self from main port
198+
if (port._portOnDisconnect) {
201199
try { port.onDisconnect.removeListener(port._portOnDisconnect); }
202200
catch(e) { console.warn("[background] Error removing _portOnDisconnect on main port disconnect:", e); }
203201
}
@@ -218,10 +216,8 @@ async function executeApi(session, port, config) {
218216
console.log(
219217
`[background] executeApi called for model: ${session.modelName}, apiMode: ${session.apiMode}`,
220218
)
221-
// Use the new helper function for session and config details
222219
console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
223220
console.debug('[background] Full config details (redacted):', redactSensitiveFields(config))
224-
// Specific redaction for session.apiMode if it exists, as it's part of the session object
225221
if (session.apiMode) {
226222
console.debug('[background] Session apiMode details (redacted):', redactSensitiveFields(session.apiMode))
227223
}
@@ -481,9 +477,7 @@ Browser.runtime.onMessage.addListener(async (message, sender) => {
481477
'Original message:',
482478
message,
483479
)
484-
// Consider if a response is expected and how to send an error response
485480
if (message.type === 'FETCH') {
486-
// FETCH expects a response
487481
return [null, { message: error.message, stack: error.stack }]
488482
}
489483
}
@@ -540,13 +534,15 @@ try {
540534
const headers = details.requestHeaders
541535
let modified = false
542536
for (let i = 0; i < headers.length; i++) {
543-
const headerNameLower = headers[i]?.name?.toLowerCase(); // Apply optional chaining
544-
if (headerNameLower === 'origin') {
545-
headers[i].value = 'https://www.bing.com'
546-
modified = true
547-
} else if (headerNameLower === 'referer') {
548-
headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
549-
modified = true
537+
if (headers[i]) {
538+
const headerNameLower = headers[i].name?.toLowerCase();
539+
if (headerNameLower === 'origin') {
540+
headers[i].value = 'https://www.bing.com'
541+
modified = true
542+
} else if (headerNameLower === 'referer') {
543+
headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
544+
modified = true
545+
}
550546
}
551547
}
552548
if (modified) {
@@ -559,45 +555,73 @@ try {
559555
error,
560556
details,
561557
)
562-
return { requestHeaders: details.requestHeaders } // Return original headers on error
558+
return { requestHeaders: details.requestHeaders }
563559
}
564560
},
565561
{
566562
urls: ['wss://sydney.bing.com/*', 'https://www.bing.com/*'],
567563
types: ['xmlhttprequest', 'websocket'],
568564
},
569-
// Use 'blocking' for modifying request headers, and ensure permissions are set in manifest
570565
['blocking', 'requestHeaders'],
571566
)
572567

573568
Browser.tabs.onUpdated.addListener(async (tabId, info, tab) => {
569+
const outerTryCatchError = (error) => { // Renamed to avoid conflict with inner error
570+
console.error('[background] Error in tabs.onUpdated listener callback (outer):', error, tabId, info);
571+
};
574572
try {
575-
// Refined condition: Ensure URL exists and tab loading is complete.
576573
if (!tab.url || (info.status && info.status !== 'complete')) {
577574
console.debug(
578575
`[background] Skipping side panel update for tabId: ${tabId}. Tab URL: ${tab.url}, Info Status: ${info.status}`,
579-
)
580-
return
576+
);
577+
return;
581578
}
582579
console.debug(
583580
`[background] tabs.onUpdated event for tabId: ${tabId}, status: ${info.status}, url: ${tab.url}. Proceeding with side panel update.`,
584-
)
585-
// Use Browser.sidePanel from webextension-polyfill for consistency and cross-browser compatibility
586-
if (Browser.sidePanel) {
587-
await Browser.sidePanel.setOptions({
588-
tabId,
589-
path: 'IndependentPanel.html',
590-
enabled: true,
591-
})
592-
console.debug(`[background] Side panel options set for tab ${tabId} using Browser.sidePanel`)
593-
} else {
594-
// Log if Browser.sidePanel is somehow not available (polyfill should generally handle this)
595-
console.warn('[background] Browser.sidePanel API not available. Side panel options not set.')
581+
);
582+
583+
let sidePanelSet = false;
584+
try {
585+
if (Browser.sidePanel && typeof Browser.sidePanel.setOptions === 'function') {
586+
await Browser.sidePanel.setOptions({
587+
tabId,
588+
path: 'IndependentPanel.html',
589+
enabled: true,
590+
});
591+
console.debug(`[background] Side panel options set for tab ${tabId} using Browser.sidePanel`);
592+
sidePanelSet = true;
593+
}
594+
} catch (browserError) {
595+
console.warn('[background] Browser.sidePanel.setOptions failed:', browserError.message);
596+
// Fallback will be attempted below if sidePanelSet is still false
597+
}
598+
599+
if (!sidePanelSet) {
600+
// eslint-disable-next-line no-undef
601+
if (typeof chrome !== 'undefined' && chrome.sidePanel && typeof chrome.sidePanel.setOptions === 'function') {
602+
console.debug('[background] Attempting chrome.sidePanel.setOptions as fallback.');
603+
try {
604+
// eslint-disable-next-line no-undef
605+
await chrome.sidePanel.setOptions({
606+
tabId,
607+
path: 'IndependentPanel.html',
608+
enabled: true,
609+
});
610+
console.debug(`[background] Side panel options set for tab ${tabId} using chrome.sidePanel`);
611+
sidePanelSet = true;
612+
} catch (chromeError) {
613+
console.error('[background] chrome.sidePanel.setOptions also failed:', chromeError.message);
614+
}
615+
}
596616
}
597-
} catch (error) {
598-
console.error('[background] Error in tabs.onUpdated listener callback:', error, tabId, info)
617+
618+
if (!sidePanelSet) {
619+
console.warn('[background] SidePanel API (Browser.sidePanel or chrome.sidePanel) not available or setOptions failed in this browser. Side panel options not set for tab:', tabId);
620+
}
621+
} catch (error) { // This is the outer try-catch from the original code
622+
outerTryCatchError(error);
599623
}
600-
})
624+
});
601625
} catch (error) {
602626
console.error('[background] Error setting up webRequest or tabs listeners:', error)
603627
}

0 commit comments

Comments
 (0)