fix(follow-up): restore custom HTTPS backend reachability via optional host permissions#13
Merged
DYAI2025 merged 1 commit intoMay 4, 2026
Conversation
Contributor
Reviewer's GuideAdds optional host permissions and a runtime permission flow so the extension can reach user-configured HTTPS backends, along with tests and mocks for the new permissions behavior. Class diagram for popup module host permission logicclassDiagram
class PopupModule {
+applyServerForm(input)
}
class EnsureHostPermission {
+ensureHostPermission(serverUrl)
}
class ChromePermissions {
+contains(params)
+request(params)
}
class ChromeRuntime {
+sendMessage(message)
}
class ConfigRepository {
+saveConfig(config)
}
PopupModule --> EnsureHostPermission : uses
PopupModule --> ConfigRepository : calls_saveConfig
PopupModule --> ChromeRuntime : sendMessage_CONFIG_UPDATED
EnsureHostPermission --> ChromePermissions : contains_and_request
note for PopupModule "Represents popup_js exported function applyServerForm"
note for EnsureHostPermission "Encapsulates host permission checking and requesting before saving config"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider restricting
ensureHostPermissionto only request permissions forhttps:URLs and/or explicitly skipping origins already covered by statichost_permissions, to avoid unnecessary permission prompts for unsupported or already-allowed schemes. - It might be helpful to wrap the
new URL(trimmed)call inensureHostPermissionwith a try/catch and throw a more explicit configuration error, so we don’t rely on the genericURLparsing exception path for invalidserverUrlvalues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider restricting `ensureHostPermission` to only request permissions for `https:` URLs and/or explicitly skipping origins already covered by static `host_permissions`, to avoid unnecessary permission prompts for unsupported or already-allowed schemes.
- It might be helpful to wrap the `new URL(trimmed)` call in `ensureHostPermission` with a try/catch and throw a more explicit configuration error, so we don’t rely on the generic `URL` parsing exception path for invalid `serverUrl` values.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e07afd8d4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+27
to
28
| await ensureHostPermission(input.serverUrl); | ||
| await saveConfig({ serverUrl: input.serverUrl, apiKey: input.apiKey }); |
There was a problem hiding this comment.
applyServerForm now calls ensureHostPermission before saveConfig, so parseable-but-invalid inputs (for example https://api.example.com/v1, which saveConfig rejects because only origin or /api paths are allowed) can still trigger and grant a host-permission prompt before the save fails. That leaves an unnecessary persistent permission on the extension even though the configuration was not accepted, which is both confusing and a permission-scope regression for users who mistype URLs.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Pull request overview
Restores reachability to user-configured HTTPS backends by introducing runtime-granted host permissions, avoiding broad static host access while keeping the existing configurable serverUrl model.
Changes:
- Added
optional_host_permissions(https://*/*) to enable origin-scoped permission grants at runtime. - Added
ensureHostPermission(serverUrl)and wired it intoapplyServerFormto request origin access when needed. - Extended the Chrome test mock and added integration tests covering permission request/deny flows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| extension/manifest.json | Adds optional HTTPS host permissions (and reformats content script arrays). |
| extension/popup.js | Adds runtime permission check/request before saving server config. |
| extension/tests/mocks/chrome.js | Adds chrome.permissions.contains/request to the test mock. |
| extension/tests/integration/popup.test.js | Adds integration tests for requesting/denying host permissions on save. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function ensureHostPermission(serverUrl) { | ||
| const trimmed = String(serverUrl || '').trim(); | ||
| if (!trimmed) return; | ||
| const u = new URL(trimmed); |
| const u = new URL(trimmed); | ||
| const pattern = `${u.origin}/*`; | ||
| const has = await chrome.permissions.contains({ origins: [pattern] }); | ||
| if (has) return; |
Comment on lines
+66
to
+69
| permissions: { | ||
| contains: vi.fn(async ({ origins }) => origins.every((origin) => origin.startsWith('http://localhost') || origin.startsWith('http://127.0.0.1') || origin.startsWith('https://web.whatsapp.com'))), | ||
| request: vi.fn(async () => true), | ||
| }, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
https://*/*fromhost_permissionsleft the extension unable to reach user-configured HTTPS backends because runtimefetchcalls require host access.serverUrlvalues via the UI and config model, so a user-granted flow is needed to enable reachability for those origins without reintroducing broad static host permissions.Description
optional_host_permissionswith"https://*/*"toextension/manifest.jsonto enable runtime-granted host access via the Permissions API.ensureHostPermission(serverUrl)inextension/popup.jsand call it fromapplyServerForm, which derives the origin, checkschrome.permissions.contains, and requests origin-scoped access viachrome.permissions.request, throwing a clear error if denied.extension/tests/mocks/chrome.jswithpermissions.containsandpermissions.requesthooks so permission flows can be exercised deterministically.extension/tests/integration/popup.test.jsto verify thatapplyServerFormrequests the correct origin-scoped permission and fails when the user denies it.Testing
cd extension && npm test -- --run tests/integration/popup.test.js, and the suite passed (5/5 tests).chrome.permissions.contains/requestand assert correct calls and error handling.Codex Task
Summary by Sourcery
Gate custom HTTPS backend usage behind runtime host-permission checks and user-granted origin access.
New Features:
Bug Fixes:
Enhancements:
Tests: