Conversation
unifolio0
temporarily deployed
to
PREVIEW_ENV
GitHub Actions
Inactive
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
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthroughdevelop 대상 PR에 대해 Preview 배포 및 정리 워크플로우를 추가하고, VITE_BASE_PATH 환경변수에 따라 앱의 라우팅, 번들 베이스, i18n 로드 경로 및 여러 URL 생성 로직을 베이스패스에 맞게 조정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant GitHub as GitHub Actions
participant Builder as CI (Node)
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->>Builder: 코드 체크아웃 및 .env 생성 (VITE_BASE_PATH 포함) 및 빌드
Builder->>GitHub: 빌드 결과 반환
GitHub->>S3: 빌드 아티팩트 업로드 (pr-{PR_NUMBER} 경로) 및 oauth-handler 업로드
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->>S3: pr-{PR_NUMBER} 경로 삭제
GitHub->>CloudFront: 관련 캐시 무효화 (선택적)
GitHub->>PR: 정리 완료 댓글 작성
PR-->>User: 정리 알림
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 배포 완료!
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| 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.
VITE_BASE_PATH를 기반으로 pathPrefix를 생성하는 로직이 여러 파일(debate_template.ts, DebateVotePage.tsx, arrayEncoding.ts)에서 동일하게 반복되고 있습니다. 이를 공통 유틸리티 함수로 분리하여 관리하는 것을 권장합니다.
![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
🧹 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.
| 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).
i-meant-to-be
left a comment
i-meant-to-be
left a comment
There was a problem hiding this comment.
확인했습니다! 작업 환경이 달라 코드 파악하시는 데 시간이 좀 걸리셨을 텐데, 직접 처리하시느라 고생하셨어요. 중요한 리뷰 한 건이 있어 RC 남기고 갑니다.
| <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> |
There was a problem hiding this comment.
권장: state 검증 로직 추가
CodeRabbit이 짚어준 내용과 동일합니다. 공격자가 "https://preview.debate-timer.com/oauth?client_id=...&redirect_uri=https://preview.debate-timer.com/oauth&state=https://evil.com" 등의 URL로 우리 팀원을 공격한다면, 팀원의 OAuth 관련 민감 정보가 유출될 가능성이 있어 보여요. state 변수에 대한 검증 로직을 추가하는 것이 좋아 보입니다.
아래는 Gemini가 제안해 준 검증 코드예요:
// 보안 패치: state가 'pr-숫자' 형식이거나, '/'로 시작하는 상대 경로인지 검증
// 예: "pr-123" 또는 "/pr-123" 만 허용
var validStateRegex = /^pr-\d+$/;
if (state && validStateRegex.test(state)) {
// 검증 통과 시에만 이동
params.delete('state');
// 상대 경로를 명시적으로 사용하여 외부 도메인 이동 차단
window.location.replace('/' + state + '/oauth?' + params.toString());
} else {
// 검증 실패 시 에러 처리 또는 메인으로 이동
console.error('Invalid state parameter');
document.body.innerHTML = "<h1>잘못된 접근입니다. (Invalid OAuth State)</h1>";
}
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@i-meant-to-be 리뷰 반영했습니다!
| <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> |
unifolio0
temporarily deployed
to
PREVIEW_ENV
GitHub Actions
Inactive
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
비토 !! 제안했던 작업 선뜻 맡아서 작업해 주셔서 정말 감사합니다 .. 덕분에 생산성이 더 증가될 것 같숨다 .. !!!!!!
unifolio0
temporarily deployed
to
PREVIEW_ENV
GitHub Actions
Inactive
unifolio0
temporarily deployed
to
PREVIEW_ENV
GitHub Actions
Inactive
|
🧹 Preview 배포가 정리되었습니다. |
3 participants
🚩 연관 이슈
closed #421
📝 작업 내용
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
New Features
New Features / 배포 설정
Improvements
Refactor