fix: skip upgrade-insecure-requests CSP for external HTTP backends#7714
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 536dd7a0de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
536dd7a to
bebead0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bebead053e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
When an external goosed backend is configured with an HTTP URL, the CSP upgrade-insecure-requests directive causes Chromium to silently rewrite requests to HTTPS. Since the remote server typically does not support TLS, the upgraded requests fail with 'Error: Failed to fetch'. Loopback addresses (127.0.0.1/localhost) are exempt from the upgrade per the CSP spec, so the built-in local backend is unaffected. This change extracts the CSP building logic into a testable utility (csp.ts) and conditionally omits upgrade-insecure-requests when the external backend URL uses plain HTTP. Fixes: block#7692 Signed-off-by: fresh3nough <fresh3nough@users.noreply.github.com> Signed-off-by: fre <anonwurcod@proton.me>
bebead0 to
4dee596
Compare
lifeizhou-ap
left a comment
There was a problem hiding this comment.
Thanks for fixing this!
|
np! |
* main: (191 commits) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) ...
* main: (65 commits) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) ...
…oken-retry * origin/main: (21 commits) Remove java/.ai-usage-marker directory (#7925) test(acp): add terminal delegation fixtures and fix shell singleton (#7923) fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892) feat: persist GooseMode per-session via session DB (#7854) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) ...
Summary
When an external goosed backend is configured with a plain HTTP URL, the CSP
upgrade-insecure-requestsdirective causes Chromium to silently rewrite those requests to HTTPS. Since the remote server typically does not support TLS, the upgraded requests fail with "Error: Failed to fetch".This change extracts the CSP building logic from
main.tsinto a dedicated, testable utility (ui/desktop/src/utils/csp.ts) and conditionally omitsupgrade-insecure-requestswhen the external backend URL uses plain HTTP. Loopback addresses (127.0.0.1/localhost) are exempt from the upgrade per the CSP spec, so the built-in local backend is unaffected.Type of Change
AI Assistance
Testing
ui/desktop/src/utils/__tests__/csp.test.tscovering:buildConnectSrc: default sources, external origin inclusion/exclusion, invalid URL handlingshouldUpgradeInsecureRequests: HTTP vs HTTPS external backends, disabled/missing configs, edge casesbuildCSP: end-to-end CSP string generation with and without the upgrade directiveRelated Issues
Fixes #7692