Conversation
- Created `functions/audio-proxy.js` to proxy audio requests from `elearning.hakka.gov.tw`. - Added `applyAudioProxy` helper in `main.js` and updated all audio URL construction points. - Updated `service-worker.js` to support caching of proxied audio URLs. - Verified fix with Playwright tests. Co-authored-by: Aiuanyu <93833368+Aiuanyu@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where audio resources from a third-party domain were blocked by modern browsers due to Cross-Origin-Resource-Policy (CORP) headers. It introduces a server-side proxy using Cloudflare Pages Functions to fetch these audio files and serve them with appropriate headers, thereby enabling their playback within the application. The changes ensure that the application can reliably load and play audio, while also maintaining PWA caching capabilities for offline access. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying hakspring with
|
| Latest commit: |
c0c54da
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://395dd43e.hakspring.pages.dev |
| Branch Preview URL: | https://fix-audio-corp-issue-4965806.hakspring.pages.dev |
PR Review: Fix audio playback blocked by CORP policyGood approach overall — using a Cloudflare Pages Function as a server-side proxy is the right solution for a third-party CORP issue you can't control. Here are my findings:
|
| Area | Status |
|---|---|
| Core approach (CF Pages Function proxy) | ✅ Correct |
| Domain allowlist security | ✅ Good |
| Range header forwarding | |
| CORS preflight (OPTIONS) | |
| Error response hardening | ℹ️ Minor improvement possible |
| Content-type validation | ℹ️ Nice-to-have |
| Local dev experience | ℹ️ Needs documentation |
| Service worker update | ✅ Correct |
| server.log removal | ✅ Good cleanup |
The Range header issue is the most impactful — I'd recommend addressing it before merging to avoid broken seeking on audio elements.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a server-side proxy to resolve a Cross-Origin-Resource-Policy issue blocking audio playback from a third-party domain. However, the proxy implementation has several security vulnerabilities. It is susceptible to SSRF via redirect following, leaks sensitive headers from the proxied domain, and acts as an open proxy for any file type on the target domain. It is recommended to restrict the proxy to specific file extensions, disable redirect following in the fetch call, and sanitize the headers returned to the client to enhance security.
| const response = await fetch(targetUrl, { | ||
| headers: { | ||
| 'User-Agent': request.headers.get('User-Agent') || 'HakSpring-Proxy', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The fetch call follows redirects by default. If elearning.hakka.gov.tw has an open redirect, an attacker could use it to fetch resources from other domains, bypassing the startsWith check on line 11. This is a classic SSRF bypass technique.
const response = await fetch(targetUrl, {
headers: {
'User-Agent': request.headers.get('User-Agent') || 'HakSpring-Proxy',
},
redirect: 'manual',
});| if (!targetUrl.startsWith('https://elearning.hakka.gov.tw/')) { | ||
| return new Response('Invalid target URL', { status: 403 }); | ||
| } |
There was a problem hiding this comment.
The proxy currently allows fetching any file type from elearning.hakka.gov.tw, which, combined with the permissive CORS header (*), bypasses cross-origin protections. The provided code suggestion restricts the proxy to .mp3 files and uses startsWith for domain validation. However, the startsWith check can be bypassed (e.g., https://elearning.hakka.gov.tw.evil.com/). For a more robust solution, consider parsing the targetUrl to validate its hostname directly.
// Basic security: only proxy audio files from elearning.hakka.gov.tw
const isAllowedDomain = targetUrl.startsWith('https://elearning.hakka.gov.tw/');
const isAudioFile = targetUrl.toLowerCase().endsWith('.mp3');
if (!isAllowedDomain || !isAudioFile) {
return new Response('Invalid target URL or file type', { status: 403 });
}| }); | ||
|
|
||
| // Create a new response to modify headers | ||
| const newResponse = new Response(response.body, response); |
There was a problem hiding this comment.
The proxy copies all headers from the third-party response to the client. This can leak sensitive headers like Set-Cookie, Server, or X-Powered-By. If the third-party server returns a Set-Cookie header, the browser will attempt to set that cookie for your domain (hakspring.pages.dev), which could lead to session fixation or cookie injection attacks.
| const newResponse = new Response(response.body, response); | |
| // Create a new response with only necessary headers | |
| const newResponse = new Response(response.body, { | |
| status: response.status, | |
| statusText: response.statusText, | |
| headers: { | |
| 'Content-Type': response.headers.get('Content-Type') || 'application/octet-stream', | |
| 'Content-Length': response.headers.get('Content-Length'), | |
| }, | |
| }); |
|
確實可解決。Jules 做得好！軍情緊急，先 merge，至於 review comment，留待未來處理。 |
多謝肯定！既然情況緊急，我已經完成所有必要个檢查摎提交。若係未來有需要處理个 review comment，隨時通知我。 |
1 participant
The audio resources from
elearning.hakka.gov.twwere being blocked by modern browsers due to aCross-Origin-Resource-Policy: same-siteheader. Since this is a third-party domain, we cannot change their server configuration.This PR implements a server-side proxy using Cloudflare Pages Functions. The proxy fetches the audio files and returns them with permissive headers (
Access-Control-Allow-Origin: *andCross-Origin-Resource-Policy: cross-origin), allowing the HakSpring application to load and play them.Key changes:
functions/audio-proxy.jswhich validates the target URL and streams the response with corrected headers.main.jsto route all external audio requests through/audio-proxy?url=....Fixes #215
PR created automatically by Jules for task 496580638850731905 started by @Aiuanyu