Skip to content

Commit 789d8fe

Browse files
Fix: 根據最新 PR Review 意見進行全面修正
針對 PR#863 的最新 review 意見進行了進一步的修正,主要包括: 1. **徹底解決 `chrome` 未定義問題:** * 標準化 `sidePanel` API 至 `Browser.sidePanel`,確保 CI Lint 通過。 2. **改進敏感資料編輯:** * 在 `src/background/index.mjs` 中實現並使用了遞迴的 `redactSensitiveFields` 輔助函數,以更全面地編輯日誌中的敏感資訊 (API keys, tokens 等)。 3. **修復 Promise 競爭條件與潛在記憶體洩漏:** * 在 `src/content-script/index.jsx` 的 `prepareForJumpBackNotification` 中,為 Claude 和 Kimi 的 token 輪詢邏輯引入了 `cleanup` 函數,以正確管理和清除定時器,防止競爭條件和記憶體洩漏。 4. **處理日誌級別一致性:** * 將 `src/background/index.mjs` 中 `setPortProxy` 函數內一處 `console.log` 修改為 `console.debug`。 5. **確認 `Origin` header 修改日誌:** * 確認 `onBeforeSendHeaders` 中對 `Origin` header 的修改已能正確觸發日誌記錄。 6. **採納 CodeRabbit Nitpicks 和建議:** * **監聽器清理:** 確認 `setPortProxy` 中監聽器清理邏輯的穩健性。 * **重連配置:** 將 `setPortProxy` 中的重連參數(最大次數、延遲、退避乘數)定義為 `RECONNECT_CONFIG` 常數。 * **可選鏈接:** 在 `src/background/index.mjs` 和 `src/content-script/index.jsx` 的建議位置應用了 `?.` 可選鏈接。 所有更改均已通過 `npm run lint`。這些修正旨在進一步提高擴充功能的穩定性、安全性和程式碼品質。
1 parent d174940 commit 789d8fe

File tree

2 files changed

+147
-103
lines changed

2 files changed

+147
-103
lines changed

src/background/index.mjs

Lines changed: 108 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,79 @@ import { generateAnswersWithMoonshotCompletionApi } from '../services/apis/moons
5151
import { generateAnswersWithMoonshotWebApi } from '../services/apis/moonshot-web.mjs'
5252
import { isUsingModelName } from '../utils/model-name-convert.mjs'
5353

54+
const RECONNECT_CONFIG = {
55+
MAX_ATTEMPTS: 5,
56+
BASE_DELAY_MS: 1000, // Base delay in milliseconds
57+
BACKOFF_MULTIPLIER: 2, // Multiplier for exponential backoff
58+
};
59+
60+
const SENSITIVE_KEYWORDS = [
61+
'apikey', // Covers apiKey, customApiKey, claudeApiKey, etc.
62+
'token', // Covers accessToken, refreshToken, etc.
63+
'secret',
64+
'password',
65+
'kimimoonshotrefreshtoken',
66+
];
67+
68+
function redactSensitiveFields(obj, recursionDepth = 0, maxDepth = 5) {
69+
if (recursionDepth > maxDepth) {
70+
// Prevent infinite recursion on circular objects or excessively deep structures
71+
return 'REDACTED_TOO_DEEP';
72+
}
73+
// Handle null, primitives, and functions directly
74+
if (obj === null || typeof obj !== 'object') {
75+
return obj;
76+
}
77+
78+
// Create a new object or array to store redacted fields, ensuring original is not modified
79+
const redactedObj = Array.isArray(obj) ? [] : {};
80+
81+
for (const key in obj) {
82+
// Ensure we're only processing own properties
83+
if (Object.prototype.hasOwnProperty.call(obj, key)) {
84+
const lowerKey = key.toLowerCase();
85+
let isSensitive = false;
86+
for (const keyword of SENSITIVE_KEYWORDS) {
87+
if (lowerKey.includes(keyword)) {
88+
isSensitive = true;
89+
break;
90+
}
91+
}
92+
93+
if (isSensitive) {
94+
redactedObj[key] = 'REDACTED';
95+
} 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+
} else {
99+
// Otherwise, copy the value as is
100+
redactedObj[key] = obj[key];
101+
}
102+
}
103+
}
104+
return redactedObj;
105+
}
106+
54107
function setPortProxy(port, proxyTabId) {
55108
try {
56109
console.debug(`[background] Attempting to connect to proxy tab: ${proxyTabId}`)
57-
// Define listeners here so they can be referenced for removal
58-
// These will be port-specific if setPortProxy is called for different ports.
59-
// However, a single port object is typically used per connection instance from the other side.
60-
// The issue arises if `setPortProxy` is called multiple times on the *same* port object for reconnections.
61110

62-
// Ensure old listeners on port.proxy are removed if it exists (e.g. from a previous failed attempt on this same port object)
111+
// Ensure old listeners on port.proxy are removed if it exists
63112
if (port.proxy) {
64113
try {
65114
if (port._proxyOnMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
66115
if (port._proxyOnDisconnect) port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
67116
} catch(e) {
68-
console.warn('[background] Error removing old listeners from previous port.proxy:', e);
117+
console.warn('[background] Error removing old listeners from previous port.proxy instance:', e);
69118
}
70119
}
71-
// Also remove listeners from the main port object that this function might have added
120+
// Also remove listeners from the main port object that this function might have added in a previous call for this port instance
72121
if (port._portOnMessage) port.onMessage.removeListener(port._portOnMessage);
73122
if (port._portOnDisconnect) port.onDisconnect.removeListener(port._portOnDisconnect);
74123

75124

76125
port.proxy = Browser.tabs.connect(proxyTabId, { name: 'background-to-content-script-proxy' })
77-
console.log(`[background] Successfully connected to proxy tab: ${proxyTabId}`)
126+
console.debug(`[background] Successfully connected to proxy tab: ${proxyTabId}`)
78127
port._reconnectAttempts = 0 // Reset retry count on successful connection
79128

80129
port._proxyOnMessage = (msg) => {
@@ -90,64 +139,75 @@ function setPortProxy(port, proxyTabId) {
90139
}
91140
}
92141

93-
const MAX_RECONNECT_ATTEMPTS = 5;
94-
95142
port._proxyOnDisconnect = () => {
96143
console.warn(`[background] Proxy tab ${proxyTabId} disconnected.`)
97144

98-
// Cleanup this specific proxy's listeners
99-
if (port.proxy) {
100-
port.proxy.onMessage.removeListener(port._proxyOnMessage);
101-
port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect); // remove self
145+
// Cleanup this specific proxy's listeners before setting port.proxy to null
146+
if (port.proxy) { // Check if port.proxy is still valid
147+
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); }
150+
}
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); }
154+
}
102155
}
103156
port.proxy = null // Clear the old proxy
104157

105158
port._reconnectAttempts = (port._reconnectAttempts || 0) + 1;
106-
if (port._reconnectAttempts > MAX_RECONNECT_ATTEMPTS) {
107-
console.error(`[background] Max reconnect attempts (${MAX_RECONNECT_ATTEMPTS}) reached for tab ${proxyTabId}. Giving up.`);
108-
// Important: also clean up listeners on the main 'port' object associated with this proxy connection
109-
if (port._portOnMessage) port.onMessage.removeListener(port._portOnMessage);
110-
// Do not remove port._portOnDisconnect here as it might be the generic one for the *other* end.
111-
// This needs careful thought: is portOnDisconnect for THIS proxy instance or for the overall port?
112-
// Assuming port.onDisconnect is for the connection that initiated this proxy.
113-
// If that disconnects, it should clean up its own _portOnMessage and _proxyOnMessage etc.
159+
if (port._reconnectAttempts > RECONNECT_CONFIG.MAX_ATTEMPTS) {
160+
console.error(`[background] Max reconnect attempts (${RECONNECT_CONFIG.MAX_ATTEMPTS}) reached for tab ${proxyTabId}. Giving up.`);
161+
if (port._portOnMessage) {
162+
try { port.onMessage.removeListener(port._portOnMessage); }
163+
catch(e) { console.warn("[background] Error removing _portOnMessage on max retries:", e); }
164+
}
165+
// Note: _portOnDisconnect on the main port should remain to handle its eventual disconnection.
114166
return;
115167
}
116168

117-
const delay = Math.pow(2, port._reconnectAttempts - 1) * 1000; // Exponential backoff
169+
const delay = Math.pow(RECONNECT_CONFIG.BACKOFF_MULTIPLIER, port._reconnectAttempts - 1) * RECONNECT_CONFIG.BASE_DELAY_MS;
118170
console.log(`[background] Attempting reconnect #${port._reconnectAttempts} in ${delay / 1000}s for tab ${proxyTabId}.`)
119171

120172
setTimeout(() => {
121173
console.debug(`[background] Retrying connection to tab ${proxyTabId}, attempt ${port._reconnectAttempts}.`);
122-
setPortProxy(port, proxyTabId); // Reconnect (will add new listeners)
174+
setPortProxy(port, proxyTabId); // Reconnect
123175
}, delay);
124176
}
125177

126-
// This is the handler for when the *other* end of the 'port' disconnects (e.g. the popup closes)
127-
// It should clean up everything related to this 'port's proxying activity.
128178
port._portOnDisconnect = () => {
129179
console.log('[background] Main port disconnected (e.g. popup/sidebar closed). Cleaning up proxy connections and listeners.');
130-
if (port._portOnMessage) port.onMessage.removeListener(port._portOnMessage);
180+
if (port._portOnMessage) {
181+
try { port.onMessage.removeListener(port._portOnMessage); }
182+
catch(e) { console.warn("[background] Error removing _portOnMessage on main port disconnect:", e); }
183+
}
131184
if (port.proxy) {
132-
if (port._proxyOnMessage) port.proxy.onMessage.removeListener(port._proxyOnMessage);
133-
if (port._proxyOnDisconnect) port.proxy.onDisconnect.removeListener(port._proxyOnDisconnect);
185+
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); }
188+
}
189+
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); }
192+
}
134193
try {
135-
port.proxy.disconnect(); // Disconnect the connection to the content script
194+
port.proxy.disconnect();
136195
} catch(e) {
137-
console.warn('[background] Error disconnecting port.proxy:', e);
196+
console.warn('[background] Error disconnecting port.proxy on main port disconnect:', e);
138197
}
139198
port.proxy = null;
140199
}
141-
// Remove self
142-
if (port._portOnDisconnect) port.onDisconnect.removeListener(port._portOnDisconnect);
143-
// Reset for potential future use of this port object if it's somehow reused (though typically new port objects are made)
200+
if (port._portOnDisconnect) { // Remove self from main port
201+
try { port.onDisconnect.removeListener(port._portOnDisconnect); }
202+
catch(e) { console.warn("[background] Error removing _portOnDisconnect on main port disconnect:", e); }
203+
}
144204
port._reconnectAttempts = 0;
145205
}
146206

147207
port.proxy.onMessage.addListener(port._proxyOnMessage)
148-
port.onMessage.addListener(port._portOnMessage) // For messages from the other end to be proxied
149-
port.proxy.onDisconnect.addListener(port._proxyOnDisconnect) // When content script/tab proxy disconnects
150-
port.onDisconnect.addListener(port._portOnDisconnect) // When the other end (popup/sidebar) disconnects
208+
port.onMessage.addListener(port._portOnMessage)
209+
port.proxy.onDisconnect.addListener(port._proxyOnDisconnect)
210+
port.onDisconnect.addListener(port._portOnDisconnect)
151211

152212
} catch (error) {
153213
console.error(`[background] Error in setPortProxy for tab ${proxyTabId}:`, error)
@@ -158,23 +218,14 @@ async function executeApi(session, port, config) {
158218
console.log(
159219
`[background] executeApi called for model: ${session.modelName}, apiMode: ${session.apiMode}`,
160220
)
161-
console.debug('[background] Full session details:', session)
162-
// Redact sensitive config details before logging
163-
const redactedConfig = { ...config };
164-
if (redactedConfig.apiKey) redactedConfig.apiKey = 'REDACTED';
165-
if (redactedConfig.customApiKey) redactedConfig.customApiKey = 'REDACTED';
166-
if (redactedConfig.claudeApiKey) redactedConfig.claudeApiKey = 'REDACTED';
167-
if (redactedConfig.kimiMoonShotRefreshToken) redactedConfig.kimiMoonShotRefreshToken = 'REDACTED';
168-
// Add any other sensitive keys that might exist in 'config' or 'session.apiMode'
169-
if (session.apiMode && session.apiMode.apiKey) {
170-
redactedConfig.apiMode = { ...session.apiMode, apiKey: 'REDACTED' };
171-
} else if (session.apiMode) {
172-
redactedConfig.apiMode = { ...session.apiMode };
221+
// Use the new helper function for session and config details
222+
console.debug('[background] Full session details (redacted):', redactSensitiveFields(session))
223+
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
225+
if (session.apiMode) {
226+
console.debug('[background] Session apiMode details (redacted):', redactSensitiveFields(session.apiMode))
173227
}
174228

175-
176-
console.debug('[background] Redacted config details:', redactedConfig)
177-
178229
try {
179230
if (isUsingCustomModel(session)) {
180231
console.debug('[background] Using Custom Model API')
@@ -489,10 +540,11 @@ try {
489540
const headers = details.requestHeaders
490541
let modified = false
491542
for (let i = 0; i < headers.length; i++) {
492-
if (headers[i].name.toLowerCase() === 'origin') {
543+
const headerNameLower = headers[i]?.name?.toLowerCase(); // Apply optional chaining
544+
if (headerNameLower === 'origin') {
493545
headers[i].value = 'https://www.bing.com'
494546
modified = true
495-
} else if (headers[i].name.toLowerCase() === 'referer') {
547+
} else if (headerNameLower === 'referer') {
496548
headers[i].value = 'https://www.bing.com/search?q=Bing+AI&showconv=1&FORM=hpcodx'
497549
modified = true
498550
}
@@ -539,21 +591,8 @@ try {
539591
})
540592
console.debug(`[background] Side panel options set for tab ${tabId} using Browser.sidePanel`)
541593
} else {
542-
// Fallback or log if Browser.sidePanel is somehow not available (though polyfill should handle it)
543-
console.debug('[background] Browser.sidePanel API not available. Attempting chrome.sidePanel as fallback.')
544-
// Keeping the original chrome check as a fallback, though ideally Browser.sidePanel should work.
545-
// eslint-disable-next-line no-undef
546-
if (chrome && chrome.sidePanel) {
547-
// eslint-disable-next-line no-undef
548-
await chrome.sidePanel.setOptions({
549-
tabId,
550-
path: 'IndependentPanel.html',
551-
enabled: true,
552-
})
553-
console.debug(`[background] Side panel options set for tab ${tabId} using chrome.sidePanel`)
554-
} else {
555-
console.debug('[background] chrome.sidePanel API also not available.')
556-
}
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.')
557596
}
558597
} catch (error) {
559598
console.error('[background] Error in tabs.onUpdated listener callback:', error, tabId, info)

0 commit comments

Comments
 (0)