Conversation
Signed-off-by: kryptocodes <srivatsantb@gmail.com>
📝 WalkthroughWalkthroughAdds a proxy configuration feature: new REST endpoints (GET/PUT/DELETE) with file-backed persistence and in-memory caching, a Chrome MV3 extension that polls the API and applies browser proxy/auth settings, Docker/runtime flags to load the extension, OpenAPI + generated client/server updates, and README documentation additions. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as "API Server"
participant FS as "File System"
participant Ext as "Browser Extension"
participant Browser as "Chromium"
participant Proxy as "Proxy Server"
Client->>API: PUT /proxy/config (host,port,creds)
API->>FS: write /chromium/proxy-config.json
API->>API: update in-memory cache
API-->>Client: 200 ProxyConfig
Ext->>API: GET /proxy/config (init / poll)
API-->>Ext: ProxyConfig
Ext->>Ext: apply proxy settings & cache creds
Ext->>Browser: set chrome.proxy.settings (fixed_servers)
Browser->>Proxy: proxied request (+ auth if provided)
Proxy-->>Browser: response
Client->>API: DELETE /proxy/config
API->>FS: remove config file
API->>API: clear cache
API-->>Client: 204 No Content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@server/cmd/api/api/proxy.go`:
- Around line 65-71: The port validation currently only checks for nil or zero
and allows negative or >65535 values; update the cfg.Port validation (the same
block that returns oapi.SetProxyConfig400JSONResponse) to also reject values
outside the valid TCP port range (1–65535) by checking *cfg.Port >= 1 &&
*cfg.Port <= 65535 and returning a BadRequest response
(oapi.SetProxyConfig400JSONResponse with an appropriate message like "port must
be between 1 and 65535") when the check fails.
- Around line 95-103: The proxy config is being written with world-readable
permissions; update the file and directory creation calls so credentials are
locked down: change the os.WriteFile call that writes proxyConfigPath from mode
0o644 to 0o600 (or 0o640 if group read is required) and tighten the os.MkdirAll
directory mode (e.g., from 0o755 to 0o700) to reduce access; locate these
changes around the os.MkdirAll("/chromium", ...) and
os.WriteFile(proxyConfigPath, data, ...) lines in proxy.go and adjust the octal
permission arguments accordingly.
In `@server/lib/oapi/oapi.go`:
- Around line 497-520: ProxyConfig currently exposes credentials (Password and
Username) in API responses; remove or mark them write-only so GET responses
don't return secrets. Update the OpenAPI schema (server/openapi.yaml) to mark
password (and username if not needed in responses) as writeOnly or create a
separate response schema that omits ProxyConfig.Password and
ProxyConfig.Username, then regenerate/update the generated type (ProxyConfig) or
add a ResponseProxyConfig type used for GETs to ensure credentials are never
serialized in responses or logs.
🧹 Nitpick comments (4)
images/chromium-headful/run-docker.sh (1)
22-27: Consider validating required proxy variables.When
PROXY_ENABLED=true, the script logsPROXY_HOST:PROXY_PORTbut doesn't validate that these variables are actually set. If they're empty, Chrome will load the extension but the proxy configuration will be invalid.♻️ Proposed validation
# Add proxy extension flags if proxy is enabled if [[ "${PROXY_ENABLED:-}" == "true" ]]; then + if [[ -z "${PROXY_HOST:-}" || -z "${PROXY_PORT:-}" ]]; then + echo "Error: PROXY_HOST and PROXY_PORT are required when PROXY_ENABLED=true" + exit 1 + fi echo "Proxy enabled: $PROXY_HOST:$PROXY_PORT" CHROMIUM_FLAGS="$CHROMIUM_FLAGS --load-extension=/home/kernel/extensions/brightdata-proxy --disable-extensions-except=/home/kernel/extensions/brightdata-proxy" fiserver/openapi.yaml (1)
1772-1801: Consider makinghostandportrequired fields.The
ProxyConfigschema has no required fields, which means a PUT request with an empty object{}would be valid. The background script checks forconfig.hostandconfig.portbefore applying, but having schema-level validation would provide clearer API contracts and better error messages.♻️ Proposed schema change
ProxyConfig: type: object description: Proxy configuration for the browser + required: + - host + - port properties: host: type: stringbrightdata-proxy-extension/background.js (1)
31-47: Add error handling forchrome.proxy.settings.set.The callback for
chrome.proxy.settings.setonly logs success but doesn't checkchrome.runtime.lastErrorfor failures. If setting the proxy fails (e.g., invalid configuration), there's no indication.♻️ Proposed fix
chrome.proxy.settings.set({ value: { mode: "fixed_servers", rules: { singleProxy: { scheme: config.scheme || "http", host: config.host, port: config.port }, bypassList: config.bypassList || ["localhost", "127.0.0.1"] } }, scope: "regular" }, () => { + if (chrome.runtime.lastError) { + console.error("Failed to set proxy:", chrome.runtime.lastError.message); + return; + } console.log("Proxy configured:", config.host + ":" + config.port); });server/cmd/api/api/proxy.go (1)
20-53: Avoid holding the RLock during file I/O and refresh the cache after load.Line 24 keeps the read lock while hitting disk and parsing JSON, which can block writers longer than needed. Also, a successful file read doesn’t refresh the in-memory cache, so repeated GETs always re-read the file.
♻️ Proposed refactor
- proxyConfigMu.RLock() - defer proxyConfigMu.RUnlock() - - // If we have a cached config, return it - if proxyConfig != nil { - log.Info("returning cached proxy config", "host", stringVal(proxyConfig.Host)) - return oapi.GetProxyConfig200JSONResponse(*proxyConfig), nil - } + proxyConfigMu.RLock() + if proxyConfig != nil { + log.Info("returning cached proxy config", "host", stringVal(proxyConfig.Host)) + proxyConfigMu.RUnlock() + return oapi.GetProxyConfig200JSONResponse(*proxyConfig), nil + } + proxyConfigMu.RUnlock() // Try to load from file data, err := os.ReadFile(proxyConfigPath) @@ - log.Info("returning proxy config from file", "host", stringVal(cfg.Host)) + // Update cache (only if still empty) + proxyConfigMu.Lock() + if proxyConfig == nil { + cfgCopy := cfg + proxyConfig = &cfgCopy + } + proxyConfigMu.Unlock() + + log.Info("returning proxy config from file", "host", stringVal(cfg.Host)) return oapi.GetProxyConfig200JSONResponse(cfg), nil
| // Validate required fields | ||
| if cfg.Host == nil || *cfg.Host == "" { | ||
| return oapi.SetProxyConfig400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "host is required"}}, nil | ||
| } | ||
| if cfg.Port == nil || *cfg.Port == 0 { | ||
| return oapi.SetProxyConfig400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "port is required"}}, nil | ||
| } |
There was a problem hiding this comment.
Validate proxy port range (1–65535).
Line 69 accepts negative or >65535 ports, which will serialize but fail downstream. Add a range check.
✅ Proposed fix
- if cfg.Port == nil || *cfg.Port == 0 {
+ if cfg.Port == nil || *cfg.Port < 1 || *cfg.Port > 65535 {
return oapi.SetProxyConfig400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "port is required"}}, nil
}🤖 Prompt for AI Agents
In `@server/cmd/api/api/proxy.go` around lines 65 - 71, The port validation
currently only checks for nil or zero and allows negative or >65535 values;
update the cfg.Port validation (the same block that returns
oapi.SetProxyConfig400JSONResponse) to also reject values outside the valid TCP
port range (1–65535) by checking *cfg.Port >= 1 && *cfg.Port <= 65535 and
returning a BadRequest response (oapi.SetProxyConfig400JSONResponse with an
appropriate message like "port must be between 1 and 65535") when the check
fails.
| if err := os.MkdirAll("/chromium", 0o755); err != nil { | ||
| log.Error("failed to create chromium dir", "error", err) | ||
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to create chromium dir"}}, nil | ||
| } | ||
|
|
||
| if err := os.WriteFile(proxyConfigPath, data, 0o644); err != nil { | ||
| log.Error("failed to write proxy config", "error", err) | ||
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write proxy config"}}, nil | ||
| } |
There was a problem hiding this comment.
Restrict proxy config file permissions to protect credentials.
Line 100 writes with 0644, which makes proxy credentials world-readable on the container filesystem. Use 0600 (or at least 0640) to limit exposure.
🔐 Proposed fix
- if err := os.WriteFile(proxyConfigPath, data, 0o644); err != nil {
+ if err := os.WriteFile(proxyConfigPath, data, 0o600); err != nil {
log.Error("failed to write proxy config", "error", err)
return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write proxy config"}}, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err := os.MkdirAll("/chromium", 0o755); err != nil { | |
| log.Error("failed to create chromium dir", "error", err) | |
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to create chromium dir"}}, nil | |
| } | |
| if err := os.WriteFile(proxyConfigPath, data, 0o644); err != nil { | |
| log.Error("failed to write proxy config", "error", err) | |
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write proxy config"}}, nil | |
| } | |
| if err := os.MkdirAll("/chromium", 0o755); err != nil { | |
| log.Error("failed to create chromium dir", "error", err) | |
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to create chromium dir"}}, nil | |
| } | |
| if err := os.WriteFile(proxyConfigPath, data, 0o600); err != nil { | |
| log.Error("failed to write proxy config", "error", err) | |
| return oapi.SetProxyConfig500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write proxy config"}}, nil | |
| } |
🤖 Prompt for AI Agents
In `@server/cmd/api/api/proxy.go` around lines 95 - 103, The proxy config is being
written with world-readable permissions; update the file and directory creation
calls so credentials are locked down: change the os.WriteFile call that writes
proxyConfigPath from mode 0o644 to 0o600 (or 0o640 if group read is required)
and tighten the os.MkdirAll directory mode (e.g., from 0o755 to 0o700) to reduce
access; locate these changes around the os.MkdirAll("/chromium", ...) and
os.WriteFile(proxyConfigPath, data, ...) lines in proxy.go and adjust the octal
permission arguments accordingly.
| // ProxyConfig Proxy configuration for the browser | ||
| type ProxyConfig struct { | ||
| // BypassList List of hosts to bypass the proxy for | ||
| BypassList *[]string `json:"bypassList,omitempty"` | ||
|
|
||
| // Host Proxy server hostname or IP address | ||
| Host *string `json:"host,omitempty"` | ||
|
|
||
| // Password Proxy authentication password | ||
| Password *string `json:"password,omitempty"` | ||
|
|
||
| // Port Proxy server port | ||
| Port *int `json:"port,omitempty"` | ||
|
|
||
| // Scheme Proxy protocol scheme | ||
| Scheme *ProxyConfigScheme `json:"scheme,omitempty"` | ||
|
|
||
| // Username Proxy authentication username | ||
| Username *string `json:"username,omitempty"` | ||
| } | ||
|
|
||
| // ProxyConfigScheme Proxy protocol scheme | ||
| type ProxyConfigScheme string | ||
|
|
There was a problem hiding this comment.
Avoid returning proxy credentials in API responses. ProxyConfig includes Password (and Username) and is used by GET/PUT responses. If not strictly required, this exposes secrets to any caller and to logs/telemetry. Consider marking password as writeOnly in server/openapi.yaml or using a response schema that omits credentials.
🤖 Prompt for AI Agents
In `@server/lib/oapi/oapi.go` around lines 497 - 520, ProxyConfig currently
exposes credentials (Password and Username) in API responses; remove or mark
them write-only so GET responses don't return secrets. Update the OpenAPI schema
(server/openapi.yaml) to mark password (and username if not needed in responses)
as writeOnly or create a separate response schema that omits
ProxyConfig.Password and ProxyConfig.Username, then regenerate/update the
generated type (ProxyConfig) or add a ResponseProxyConfig type used for GETs to
ensure credentials are never serialized in responses or logs.
Signed-off-by: kryptocodes <srivatsantb@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@brightdata-proxy-extension/background.js`:
- Around line 49-67: The onAuthRequired handler
(chrome.webRequest.onAuthRequired) currently returns proxyConfig credentials for
any auth challenge; restrict it so you only supply credentials when
details.isProxy === true and proxyConfig exists, e.g., check details.isProxy
before calling callbackFn with authCredentials, otherwise call
callbackFn({cancel:false}); optionally also validate the challenger host in
details.challenger or details.host against your configured proxy host to avoid
sending proxy credentials to origin/server challenges.
🧹 Nitpick comments (3)
brightdata-proxy-extension/background.js (2)
6-19: Add a timeout to avoid a hung fetch when the local API is down.
A short AbortController timeout keeps the MV3 service worker from waiting indefinitely.♻️ Suggested update
async function fetchProxyConfig() { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 3000); try { - const response = await fetch(CONFIG_URL); + const response = await fetch(CONFIG_URL, { signal: controller.signal }); if (!response.ok) { console.error("Failed to fetch proxy config:", response.status); return null; } return await response.json(); } catch (error) { console.error("Error fetching proxy config:", error); return null; + } finally { + clearTimeout(timeoutId); } }
21-47: Clear cached credentials when disabling the proxy and surface Chrome API errors.
Without clearingproxyConfig, stale credentials can linger; also surfacechrome.runtime.lastErrorto avoid silent failures.♻️ Suggested update
function applyProxySettings(config) { if (!config || !config.host || !config.port) { console.log("No valid proxy config, disabling proxy"); - chrome.proxy.settings.clear({ scope: "regular" }); + proxyConfig = null; + chrome.proxy.settings.clear({ scope: "regular" }, () => { + if (chrome.runtime.lastError) { + console.error("Failed to clear proxy settings:", chrome.runtime.lastError.message); + } + }); return; } proxyConfig = config; chrome.proxy.settings.set({ value: { mode: "fixed_servers", rules: { singleProxy: { scheme: config.scheme || "http", host: config.host, port: config.port }, bypassList: config.bypassList || ["localhost", "127.0.0.1"] } }, scope: "regular" }, () => { + if (chrome.runtime.lastError) { + console.error("Failed to set proxy settings:", chrome.runtime.lastError.message); + return; + } console.log("Proxy configured:", config.host + ":" + config.port); }); }server/openapi.yaml (1)
1772-1801: Consider requiringhostandportto prevent invalid configs.
This aligns the schema with the extension’s expectations and the existence of DELETE for “disable.”♻️ Suggested update
ProxyConfig: type: object description: Proxy configuration for the browser + required: [host, port] properties: host: type: string description: Proxy server hostname or IP address + minLength: 1 port: type: integer description: Proxy server port minimum: 1 maximum: 65535
| // Handle proxy authentication | ||
| chrome.webRequest.onAuthRequired.addListener( | ||
| (details, callbackFn) => { | ||
| if (proxyConfig && proxyConfig.username && proxyConfig.password) { | ||
| console.log("Auth required, providing credentials"); | ||
| callbackFn({ | ||
| authCredentials: { | ||
| username: proxyConfig.username, | ||
| password: proxyConfig.password | ||
| } | ||
| }); | ||
| } else { | ||
| console.log("Auth required but no credentials available"); | ||
| callbackFn({ cancel: false }); | ||
| } | ||
| }, | ||
| { urls: ["<all_urls>"] }, | ||
| ["asyncBlocking"] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Chrome extension webRequest onAuthRequired details.isProxy challenger property API documentation
💡 Result:
In chrome.webRequest.onAuthRequired, the event details object includes:
-
details.challenger: an object describing the server (or proxy) that is requesting authentication:challenger.host(string): hostnamechallenger.port(integer): port (developer.mozilla.org)
-
details.isProxy(boolean): indicates whether the auth challenge is coming from a proxy:true= proxy auth (Proxy-Authenticate, typically HTTP 407)false= origin/server auth (WWW-Authenticate, typically HTTP 401) (developer.mozilla.org)
Notes:
onAuthRequiredis only fired for HTTP and HTTPS/TLS proxies that require authentication (not SOCKS proxies). (developer.mozilla.org)- In Manifest V3, Chrome requires the
webRequestAuthProviderpermission to useonAuthRequired. (developer.chrome.com)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/onAuthRequired?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/onAuthRequired?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/onAuthRequired?utm_source=openai
- 4: https://developer.chrome.com/docs/extensions/reference/api/webRequest?utm_source=openai
🏁 Script executed:
# Check if there are any existing security checks in the codebase
rg -i "isProxy|challenger" brightdata-proxy-extension/Repository: reclaimprotocol/popcorn-images
Length of output: 56
🏁 Script executed:
# Look for any comments or documentation about auth handling
rg -i "auth|credential" brightdata-proxy-extension/background.js -A 2 -B 2Repository: reclaimprotocol/popcorn-images
Length of output: 575
🏁 Script executed:
rg "proxyConfig\s*=" brightdata-proxy-extension/background.js -B 3 -A 3Repository: reclaimprotocol/popcorn-images
Length of output: 367
🏁 Script executed:
rg "proxyConfig" brightdata-proxy-extension/background.js -B 2 -A 2 | head -40Repository: reclaimprotocol/popcorn-images
Length of output: 615
🏁 Script executed:
rg "proxyConfig\." brightdata-proxy-extension/background.js | sort | uniqRepository: reclaimprotocol/popcorn-images
Length of output: 227
🏁 Script executed:
# Look for the config object structure or where it's used
rg "config\." brightdata-proxy-extension/background.js -B 2 -A 2 | head -50Repository: reclaimprotocol/popcorn-images
Length of output: 646
Restrict auth handling to proxy challenges to prevent credential leakage.
onAuthRequired fires for both HTTP proxy auth challenges (HTTP 407, isProxy=true) and origin/server auth challenges (HTTP 401, isProxy=false). Without a guard on details.isProxy, proxy credentials are leaked to any website that requests authentication. Add a check for details.isProxy === true before responding with credentials, and optionally validate the challenger host matches the proxy host.
🔒 Suggested hardening
chrome.webRequest.onAuthRequired.addListener(
(details, callbackFn) => {
+ if (!details.isProxy) {
+ callbackFn({ cancel: false });
+ return;
+ }
+ if (proxyConfig?.host && details.challenger?.host && details.challenger.host !== proxyConfig.host) {
+ callbackFn({ cancel: false });
+ return;
+ }
if (proxyConfig && proxyConfig.username && proxyConfig.password) {
console.log("Auth required, providing credentials");
callbackFn({
authCredentials: {
username: proxyConfig.username,
password: proxyConfig.password
}
});
} else {
console.log("Auth required but no credentials available");
callbackFn({ cancel: false });
}
},
{ urls: ["<all_urls>"] },
["asyncBlocking"]
);🤖 Prompt for AI Agents
In `@brightdata-proxy-extension/background.js` around lines 49 - 67, The
onAuthRequired handler (chrome.webRequest.onAuthRequired) currently returns
proxyConfig credentials for any auth challenge; restrict it so you only supply
credentials when details.isProxy === true and proxyConfig exists, e.g., check
details.isProxy before calling callbackFn with authCredentials, otherwise call
callbackFn({cancel:false}); optionally also validate the challenger host in
details.challenger or details.host against your configured proxy host to avoid
sending proxy credentials to origin/server challenges.
- auto refresh/load once the config is detected Signed-off-by: kryptocodes <srivatsantb@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@brightdata-proxy-extension/background.js`:
- Around line 20-33: fetchProxyConfig currently returns null for both "no
config" and fetch errors, causing applyProxySettings(null) to clear proxy on
transient failures; change fetchProxyConfig to distinguish these cases: when
response.ok is false, if response.status indicates "no config" (e.g. 404) return
null, but for other non-ok statuses and for caught network errors return a
distinct failure value (e.g. undefined) and log the error; then update the
caller(s) that call applyProxySettings to only call it when the returned value
is not undefined (i.e. only apply when config !== undefined), so transient fetch
failures skip applying/clearing settings.
- Around line 48-74: The applyProxySettings function uses parseInt(config.port,
10) without validating the result, so you must validate the parsed port is a
finite integer in the valid TCP range (1–65535) before calling
chrome.proxy.settings.set or saving config/cachedCredentials; if the port is
invalid (NaN, non-integer, or out of range) clear the proxy
(chrome.proxy.settings.clear), set storage proxyConfig/lastConfigHash to null,
reset cachedCredentials, and optionally log the bad config, otherwise continue
to save proxyConfig/lastConfigHash, set cachedCredentials and call
chrome.proxy.settings.set with the validated integer port.
In `@README.md`:
- Around line 180-184: The Markdown table header and divider need consistent
spacing so markdownlint MD060 is satisfied: edit the table that starts with the
header "Country | Username Suffix" and the rows containing `-country-in`,
`-country-us`, and `-country-gb` to use uniform single spaces around each pipe
and ensure the separator row uses matching column widths (e.g.,
`|---------|-----------------|`) so the pipes in the header, separator, and data
rows align consistently.
| // Fetch proxy configuration from API | ||
| async function fetchProxyConfig() { | ||
| try { | ||
| const response = await fetch(CONFIG_URL); | ||
| if (!response.ok) { | ||
| console.error("Failed to fetch proxy config:", response.status); | ||
| return null; | ||
| } | ||
| return await response.json(); | ||
| } catch (error) { | ||
| console.error("Error fetching proxy config:", error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid clearing proxy settings on transient API failures.
Any fetch error currently returns null, which triggers applyProxySettings(null) and clears the proxy. This can drop proxying during temporary API outages. Distinguish “no config” from “fetch failed,” and skip apply on errors.
🛠️ Suggested fix
async function fetchProxyConfig() {
try {
const response = await fetch(CONFIG_URL);
- if (!response.ok) {
- console.error("Failed to fetch proxy config:", response.status);
- return null;
- }
+ if (response.status === 404 || response.status === 204) {
+ return null; // explicitly disabled
+ }
+ if (!response.ok) {
+ console.error("Failed to fetch proxy config:", response.status);
+ return undefined;
+ }
return await response.json();
} catch (error) {
console.error("Error fetching proxy config:", error);
- return null;
+ return undefined;
}
}
async function checkAndApplyConfig() {
const config = await fetchProxyConfig();
+ if (config === undefined) return;
const newHash = hashConfig(config);
const stored = await chrome.storage.local.get(["lastConfigHash"]);Also applies to: 76-85
🤖 Prompt for AI Agents
In `@brightdata-proxy-extension/background.js` around lines 20 - 33,
fetchProxyConfig currently returns null for both "no config" and fetch errors,
causing applyProxySettings(null) to clear proxy on transient failures; change
fetchProxyConfig to distinguish these cases: when response.ok is false, if
response.status indicates "no config" (e.g. 404) return null, but for other
non-ok statuses and for caught network errors return a distinct failure value
(e.g. undefined) and log the error; then update the caller(s) that call
applyProxySettings to only call it when the returned value is not undefined
(i.e. only apply when config !== undefined), so transient fetch failures skip
applying/clearing settings.
| // Apply proxy settings | ||
| async function applyProxySettings(config) { | ||
| if (!config || !config.host || !config.port) { | ||
| chrome.proxy.settings.clear({ scope: "regular" }); | ||
| await chrome.storage.local.set({ proxyConfig: null, lastConfigHash: null }); | ||
| cachedCredentials = null; | ||
| return; | ||
| } | ||
|
|
||
| await chrome.storage.local.set({ proxyConfig: config, lastConfigHash: hashConfig(config) }); | ||
| cachedCredentials = config; | ||
|
|
||
| chrome.proxy.settings.set({ | ||
| value: { | ||
| mode: "fixed_servers", | ||
| rules: { | ||
| singleProxy: { | ||
| scheme: config.scheme || "http", | ||
| host: config.host, | ||
| port: parseInt(config.port, 10) | ||
| }, | ||
| bypassList: config.bypassList || ["localhost", "127.0.0.1"] | ||
| } | ||
| }, | ||
| scope: "regular" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Validate proxy port before applying settings.
parseInt can yield NaN or out-of-range values, which may cause a silent failure. Validate the port before calling chrome.proxy.settings.set.
✅ Proposed validation
await chrome.storage.local.set({ proxyConfig: config, lastConfigHash: hashConfig(config) });
cachedCredentials = config;
+ const port = Number.parseInt(config.port, 10);
+ if (!Number.isInteger(port) || port < 1 || port > 65535) {
+ console.error("Invalid proxy port:", config.port);
+ return;
+ }
+
chrome.proxy.settings.set({
value: {
mode: "fixed_servers",
rules: {
singleProxy: {
scheme: config.scheme || "http",
host: config.host,
- port: parseInt(config.port, 10)
+ port
},
bypassList: config.bypassList || ["localhost", "127.0.0.1"]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Apply proxy settings | |
| async function applyProxySettings(config) { | |
| if (!config || !config.host || !config.port) { | |
| chrome.proxy.settings.clear({ scope: "regular" }); | |
| await chrome.storage.local.set({ proxyConfig: null, lastConfigHash: null }); | |
| cachedCredentials = null; | |
| return; | |
| } | |
| await chrome.storage.local.set({ proxyConfig: config, lastConfigHash: hashConfig(config) }); | |
| cachedCredentials = config; | |
| chrome.proxy.settings.set({ | |
| value: { | |
| mode: "fixed_servers", | |
| rules: { | |
| singleProxy: { | |
| scheme: config.scheme || "http", | |
| host: config.host, | |
| port: parseInt(config.port, 10) | |
| }, | |
| bypassList: config.bypassList || ["localhost", "127.0.0.1"] | |
| } | |
| }, | |
| scope: "regular" | |
| }); | |
| } | |
| // Apply proxy settings | |
| async function applyProxySettings(config) { | |
| if (!config || !config.host || !config.port) { | |
| chrome.proxy.settings.clear({ scope: "regular" }); | |
| await chrome.storage.local.set({ proxyConfig: null, lastConfigHash: null }); | |
| cachedCredentials = null; | |
| return; | |
| } | |
| await chrome.storage.local.set({ proxyConfig: config, lastConfigHash: hashConfig(config) }); | |
| cachedCredentials = config; | |
| const port = Number.parseInt(config.port, 10); | |
| if (!Number.isInteger(port) || port < 1 || port > 65535) { | |
| console.error("Invalid proxy port:", config.port); | |
| return; | |
| } | |
| chrome.proxy.settings.set({ | |
| value: { | |
| mode: "fixed_servers", | |
| rules: { | |
| singleProxy: { | |
| scheme: config.scheme || "http", | |
| host: config.host, | |
| port | |
| }, | |
| bypassList: config.bypassList || ["localhost", "127.0.0.1"] | |
| } | |
| }, | |
| scope: "regular" | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@brightdata-proxy-extension/background.js` around lines 48 - 74, The
applyProxySettings function uses parseInt(config.port, 10) without validating
the result, so you must validate the parsed port is a finite integer in the
valid TCP range (1–65535) before calling chrome.proxy.settings.set or saving
config/cachedCredentials; if the port is invalid (NaN, non-integer, or out of
range) clear the proxy (chrome.proxy.settings.clear), set storage
proxyConfig/lastConfigHash to null, reset cachedCredentials, and optionally log
the bad config, otherwise continue to save proxyConfig/lastConfigHash, set
cachedCredentials and call chrome.proxy.settings.set with the validated integer
port.
| | Country | Username Suffix | | ||
| |---------|-----------------| | ||
| | India | `-country-in` | | ||
| | United States | `-country-us` | | ||
| | United Kingdom | `-country-gb` | |
There was a problem hiding this comment.
Fix table divider spacing to satisfy markdownlint MD060.
✅ Proposed formatting fix
-|---------|-----------------|
+| ------- | ---------------- |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Country | Username Suffix | | |
| |---------|-----------------| | |
| | India | `-country-in` | | |
| | United States | `-country-us` | | |
| | United Kingdom | `-country-gb` | | |
| | Country | Username Suffix | | |
| | ------- | ---------------- | | |
| | India | `-country-in` | | |
| | United States | `-country-us` | | |
| | United Kingdom | `-country-gb` | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
181-181: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
181-181: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
181-181: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
181-181: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@README.md` around lines 180 - 184, The Markdown table header and divider need
consistent spacing so markdownlint MD060 is satisfied: edit the table that
starts with the header "Country | Username Suffix" and the rows containing
`-country-in`, `-country-us`, and `-country-gb` to use uniform single spaces
around each pipe and ensure the separator row uses matching column widths (e.g.,
`|---------|-----------------|`) so the pipes in the header, separator, and data
rows align consistently.
1 participant
Description
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.