Conversation
Summary of ChangesHello @unifolio0, 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! 이 PR은 프리뷰 배포 기능을 지원하기 위해 애플리케이션의 기본 경로(base path) 처리 방식을 개선합니다. Highlights
Changelog
Ignored Files
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
|
|
No actionable comments were generated in the recent review. 🎉 WalkthroughPR이 develop 대상 PR 이벤트에 대해 Preview 배포·정리 GitHub Actions를 추가하고, VITE_BASE_PATH 환경변수를 기반으로 URL 생성, 라우팅 및 i18n 로딩 경로를 베이스패스에 맞춰 조정합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant GitHub as GitHub Actions
participant S3 as AWS S3
participant CloudFront as CloudFront
participant PR as PR 댓글
rect rgba(100, 150, 255, 0.5)
Note over User,PR: Deploy Preview (PR 열기/업데이트)
User->>GitHub: PR 열기 또는 푸시
GitHub->>GitHub: Node.js 설정 & 코드 체크아웃
GitHub->>GitHub: .env 생성 (VITE_BASE_PATH 포함) 및 빌드
GitHub->>S3: 빌드 아티팩트 업로드 (pr-{PR_NUMBER} 경로)
GitHub->>S3: oauth-handler.html 업로드
GitHub->>CloudFront: 캐시 무효화 (PR 경로 및 /oauth)
GitHub->>PR: Preview URL 포함 댓글 생성/갱신
PR-->>User: 배포 알림
end
rect rgba(150, 200, 100, 0.5)
Note over User,PR: Cleanup Preview (PR 닫기)
User->>GitHub: PR 닫기
GitHub->>GitHub: AWS 자격증명 구성
GitHub->>S3: pr-{PR_NUMBER} 경로 삭제
GitHub->>PR: 정리 완료 댓글 작성
PR-->>User: 정리 알림
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🚀 Preview 배포 완료!
|
| const basePath = import.meta.env.VITE_BASE_PATH; | ||
| const pathPrefix = basePath && basePath !== '/' ? basePath : ''; | ||
| return `${baseUrl}${pathPrefix}/vote/${pollId}`; |
There was a problem hiding this comment.
baseUrl이 undefined일 경우(특히 개발 환경에서), voteUrl이 "undefined/vote/..."와 같은 잘못된 문자열로 생성됩니다. 또한 baseUrl에 트레일링 슬래시가 포함되어 있을 경우 경로가 중복될 수 있으므로, 다른 파일들(debate_template.ts, arrayEncoding.ts)과 마찬가지로 window.location.origin을 폴백으로 사용하고 슬래시를 정규화하는 로직이 필요합니다.
const resolvedBaseUrl = (baseUrl || window.location.origin).replace(/\/+$/, '');
const basePath = import.meta.env.VITE_BASE_PATH;
const pathPrefix = basePath && basePath !== '/' ? basePath : '';
return `${resolvedBaseUrl}${pathPrefix}/vote/${pollId}`;
| const basePath = import.meta.env.VITE_BASE_PATH; | ||
| const pathPrefix = basePath && basePath !== '/' ? basePath : ''; |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/deploy-preview.yml (2)
82-83: 프리뷰 도메인이 하드코딩되어 있습니다.
preview.debate-timer.com이 워크플로우 스크립트에 직접 하드코딩되어 있습니다. 시크릿이나 변수로 관리하면 도메인 변경 시 워크플로우 수정 없이 대응할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-preview.yml around lines 82 - 83, The preview domain is hardcoded when constructing url using prNumber; change this to use a workflow variable or secret (e.g., PREVIEW_DOMAIN) instead of the literal "https://preview.debate-timer.com", and update the url construction (where url is defined) to interpolate that env/secret value (or default) so domain changes are managed via workflow inputs/environment rather than editing the script.
29-33:.env생성 시 셸 인젝션 방지를 위해 heredoc 사용을 권장합니다.
${{ vars.ENV }}와${{ secrets.ENV }}값에 셸 특수 문자가 포함되면 예기치 않은 동작이 발생할 수 있습니다. Heredoc을 사용하면 더 안전합니다.♻️ Heredoc 방식으로 변경
- name: Setup .env run: | - echo "${{ vars.ENV }}" > .env - echo "${{ secrets.ENV }}" >> .env + cat > .env << 'ENVEOF' + ${{ vars.ENV }} + ENVEOF + cat >> .env << 'ENVEOF' + ${{ secrets.ENV }} + ENVEOF echo "VITE_BASE_PATH=/pr-${{ github.event.pull_request.number }}" >> .env🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-preview.yml around lines 29 - 33, The "Setup .env" step uses multiple echo calls that can be vulnerable to shell interpretation of special characters in ${{ vars.ENV }} and ${{ secrets.ENV }}; replace the three echo lines with a single heredoc that writes all content into .env (use a quoted delimiter like <<'EOF' to prevent the shell from interpreting special characters), include the expanded values ${{ vars.ENV }} and ${{ secrets.ENV }} and the VITE_BASE_PATH line inside the heredoc, and close with EOF so the file is created atomically and safely.src/page/DebateVotePage/DebateVotePage.tsx (1)
36-38: base path 접두사 로직이 여러 파일에 중복되어 있습니다.동일한 패턴이
arrayEncoding.ts,debate_template.ts,DebateVotePage.tsx,googleAuth.ts에 반복됩니다:const basePath = import.meta.env.VITE_BASE_PATH; const pathPrefix = basePath && basePath !== '/' ? basePath : '';공통 유틸 함수로 추출하면 유지보수성이 향상됩니다.
♻️ 유틸 함수 추출 예시
예를 들어,
src/util/basePath.ts에 아래와 같이 정의:export function getBasePathPrefix(): string { const basePath = import.meta.env.VITE_BASE_PATH; return basePath && basePath !== '/' ? basePath : ''; }그 후 각 파일에서
getBasePathPrefix()를 호출하는 방식으로 변경할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/DebateVotePage/DebateVotePage.tsx` around lines 36 - 38, Multiple files duplicate the base path prefix logic (the two-line snippet calculating pathPrefix); extract this into a shared util (e.g., export function getBasePathPrefix() in a new util file) and replace the inline logic in DebateVotePage (and the other occurrences like arrayEncoding.ts, debate_template.ts, googleAuth.ts) by importing and calling getBasePathPrefix(); update the code that builds the vote URL (currently returning `${baseUrl}${pathPrefix}/vote/${pollId}`) to use the returned prefix from getBasePathPrefix() instead of recalculating it inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cleanup-preview.yml:
- Around line 23-25: The cleanup workflow's "Delete Preview from S3" step only
removes objects from S3 but doesn't invalidate CloudFront, so cached assets can
still be served; update that step to run an AWS CloudFront invalidation after
the aws s3 rm command by calling the AWS CLI (aws cloudfront
create-invalidation) using the distribution ID from a secret (e.g.,
AWS_CLOUDFRONT_DISTRIBUTION_ID) and invalidate the preview path
/pr-<pull_request_number>/* (use github.event.pull_request.number to build the
path), and ensure the step handles the operation result (fail the job on
invalidation error or wait for completion) so the preview is fully removed from
the CDN.
In @.github/workflows/deploy-preview.yml:
- Around line 54-69: The oauth-handler.html currently reads the URLSearchParams
'state' and calls window.location.replace(state + '/oauth?' + params.toString())
without validation, enabling open-redirect; update the JS in oauth-handler.html
to validate the extracted state value against an explicit whitelist or ensure it
is a same-origin path (reject absolute URLs with other origins), only perform
window.location.replace when validation passes, and otherwise render a safe
error/notice or a fixed internal redirect; keep the existing
params.delete('state') behavior but do not redirect if validation fails.
In `@src/util/googleAuth.ts`:
- Around line 17-20: The code currently sets params.state = basePath (basePath,
params.state) which enables open-redirects because oauth-handler.html blindly
redirects using state; update oauth-handler.html to treat state only as a safe
local route by validating the received state against a whitelist/pattern (e.g.,
ensure it starts with "/pr-" or matches allowed path regex) and reject or fall
back to a safe default if it doesn't match, and keep using state for CSRF tokens
separately (do not rely on it for arbitrary redirect destinations).
---
Nitpick comments:
In @.github/workflows/deploy-preview.yml:
- Around line 82-83: The preview domain is hardcoded when constructing url using
prNumber; change this to use a workflow variable or secret (e.g.,
PREVIEW_DOMAIN) instead of the literal "https://preview.debate-timer.com", and
update the url construction (where url is defined) to interpolate that
env/secret value (or default) so domain changes are managed via workflow
inputs/environment rather than editing the script.
- Around line 29-33: The "Setup .env" step uses multiple echo calls that can be
vulnerable to shell interpretation of special characters in ${{ vars.ENV }} and
${{ secrets.ENV }}; replace the three echo lines with a single heredoc that
writes all content into .env (use a quoted delimiter like <<'EOF' to prevent the
shell from interpreting special characters), include the expanded values ${{
vars.ENV }} and ${{ secrets.ENV }} and the VITE_BASE_PATH line inside the
heredoc, and close with EOF so the file is created atomically and safely.
In `@src/page/DebateVotePage/DebateVotePage.tsx`:
- Around line 36-38: Multiple files duplicate the base path prefix logic (the
two-line snippet calculating pathPrefix); extract this into a shared util (e.g.,
export function getBasePathPrefix() in a new util file) and replace the inline
logic in DebateVotePage (and the other occurrences like arrayEncoding.ts,
debate_template.ts, googleAuth.ts) by importing and calling getBasePathPrefix();
update the code that builds the vote URL (currently returning
`${baseUrl}${pathPrefix}/vote/${pollId}`) to use the returned prefix from
getBasePathPrefix() instead of recalculating it inline.
| - name: Delete Preview from S3 | ||
| run: | | ||
| aws s3 rm s3://${{ secrets.AWS_PREVIEW_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }}/ --recursive |
There was a problem hiding this comment.
S3 삭제 후 CloudFront 캐시 무효화가 누락되었습니다.
deploy-preview.yml에서는 CloudFront 무효화를 수행하지만, 정리 워크플로우에서는 S3 객체만 삭제합니다. 캐시된 콘텐츠가 CloudFront 엣지에 남아 있어 삭제된 프리뷰가 TTL 만료 전까지 계속 제공될 수 있습니다.
🛠️ CloudFront 무효화 단계 추가
- name: Delete Preview from S3
run: |
aws s3 rm s3://${{ secrets.AWS_PREVIEW_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }}/ --recursive
+
+ - name: Invalidate CloudFront Cache
+ run: |
+ aws cloudfront create-invalidation \
+ --distribution-id ${{ secrets.AWS_PREVIEW_CLOUDFRONT_ID }} \
+ --paths "/pr-${{ github.event.pull_request.number }}/*"📝 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.
| - name: Delete Preview from S3 | |
| run: | | |
| aws s3 rm s3://${{ secrets.AWS_PREVIEW_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }}/ --recursive | |
| - name: Delete Preview from S3 | |
| run: | | |
| aws s3 rm s3://${{ secrets.AWS_PREVIEW_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }}/ --recursive | |
| - name: Invalidate CloudFront Cache | |
| run: | | |
| aws cloudfront create-invalidation \ | |
| --distribution-id ${{ secrets.AWS_PREVIEW_CLOUDFRONT_ID }} \ | |
| --paths "/pr-${{ github.event.pull_request.number }}/*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/cleanup-preview.yml around lines 23 - 25, The cleanup
workflow's "Delete Preview from S3" step only removes objects from S3 but
doesn't invalidate CloudFront, so cached assets can still be served; update that
step to run an AWS CloudFront invalidation after the aws s3 rm command by
calling the AWS CLI (aws cloudfront create-invalidation) using the distribution
ID from a secret (e.g., AWS_CLOUDFRONT_DISTRIBUTION_ID) and invalidate the
preview path /pr-<pull_request_number>/* (use github.event.pull_request.number
to build the path), and ensure the step handles the operation result (fail the
job on invalidation error or wait for completion) so the preview is fully
removed from the CDN.
| cat > /tmp/oauth-handler.html << 'OAUTH_EOF' | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head><title>Redirecting...</title></head> | ||
| <body> | ||
| <script> | ||
| var params = new URLSearchParams(window.location.search); | ||
| var state = params.get('state'); | ||
| if (state) { | ||
| params.delete('state'); | ||
| window.location.replace(state + '/oauth?' + params.toString()); | ||
| } | ||
| </script> | ||
| </body> | ||
| </html> | ||
| OAUTH_EOF |
There was a problem hiding this comment.
oauth-handler.html에 오픈 리다이렉트 취약점이 있습니다.
state 파라미터를 검증 없이 리다이렉트 대상으로 사용하고 있습니다. 공격자가 Google OAuth 콜백에 악의적인 state 값(예: https://evil.com)을 삽입하면, 인증 코드가 공격자 서버로 유출될 수 있습니다.
또한 state가 없을 경우 사용자에게 "Redirecting..." 텍스트만 표시되고 아무 동작도 하지 않습니다.
🔒 state 파라미터 검증 추가
<script>
var params = new URLSearchParams(window.location.search);
var state = params.get('state');
- if (state) {
+ if (state && /^\/pr-\d+$/.test(state)) {
params.delete('state');
window.location.replace(state + '/oauth?' + params.toString());
+ } else {
+ window.location.replace('/oauth?' + params.toString());
}
</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-preview.yml around lines 54 - 69, The
oauth-handler.html currently reads the URLSearchParams 'state' and calls
window.location.replace(state + '/oauth?' + params.toString()) without
validation, enabling open-redirect; update the JS in oauth-handler.html to
validate the extracted state value against an explicit whitelist or ensure it is
a same-origin path (reject absolute URLs with other origins), only perform
window.location.replace when validation passes, and otherwise render a safe
error/notice or a fixed internal redirect; keep the existing
params.delete('state') behavior but do not redirect if validation fails.
| const basePath = import.meta.env.VITE_BASE_PATH; | ||
| if (basePath && basePath !== '/') { | ||
| params.state = basePath; | ||
| } |
There was a problem hiding this comment.
OAuth state 파라미터를 라우팅 목적으로 사용하면 보안 위험이 있습니다.
OAuth state 파라미터는 일반적으로 CSRF 방지를 위해 사용됩니다. 여기서 basePath를 전달하는 용도로 사용하고 있는데, deploy-preview.yml의 oauth-handler.html이 state 값을 기반으로 무조건 리다이렉트하므로 오픈 리다이렉트 취약점이 발생합니다.
공격자가 https://preview.debate-timer.com/oauth?code=AUTH_CODE&state=https://evil.com 같은 URL을 구성하면, oauth-handler.html이 https://evil.com/oauth?code=AUTH_CODE로 리다이렉트하여 인증 코드가 유출될 수 있습니다.
oauth-handler.html에서 state 값이 허용된 경로 패턴(예: /pr- 접두사)인지 검증해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util/googleAuth.ts` around lines 17 - 20, The code currently sets
params.state = basePath (basePath, params.state) which enables open-redirects
because oauth-handler.html blindly redirects using state; update
oauth-handler.html to treat state only as a safe local route by validating the
received state against a whitelist/pattern (e.g., ensure it starts with "/pr-"
or matches allowed path regex) and reject or fall back to a safe default if it
doesn't match, and keep using state for CSRF tokens separately (do not rely on
it for arbitrary redirect destinations).
🚩 연관 이슈
closed #421
📝 작업 내용
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit