-
Notifications
You must be signed in to change notification settings - Fork 1
[Fix] preview.yml 수정 #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix] preview.yml 수정 #204
The head ref may contain hidden characters: "193-fix-previewyml-\uC218\uC815"
Conversation
개요Walkthrough이 풀 리퀘스트는 여러 파일에 걸쳐 메시지 처리 방식을 변경하고 있습니다. 주요 변경 사항은 모바일 앱의 여러 라우트 파일에서 Changes
Suggested reviewers
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🚀 Preview URLBranch: 193-fix-previewyml-수정 Preview URL: https://codeit.click?pr=204 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/next.config.mjs (1)
75-84: 리라이트 규칙이 올바르게 구현되었습니다.PR 프리뷰 URL의 경로를 적절하게 재작성하는 규칙이 추가되었습니다.
/pr-{number}/path와 같은 URL 요청을/path로 변환하여 최신 코드 변경사항이 프리뷰에 정확하게 반영되도록 합니다.다만 유지보수성을 위해 리라이트 규칙에 대한 설명을 주석으로 추가하면 좋을 것 같습니다:
async rewrites() { return { beforeFiles: [ + // Transforms preview URLs from /pr-{number}/path to /path + // This ensures that the preview reflects the latest changes { source: "/pr-:number/:path*", destination: "/:path*", }, ], }; },apps/mobile/app/(route)/meetings.tsx (1)
Line range hint
1-19: 공통 로직 추출 고려여러 라우트 파일에서 동일한 WebView 메시지 처리 로직이 반복되고 있습니다. 코드 재사용성과 유지보수성을 높이기 위해 공통 컴포넌트나 훅으로 추출하는 것이 좋겠습니다.
다음과 같은 커스텀 훅 생성을 제안합니다:
// hooks/useWebViewMessageHandler.ts import { WebViewMessageEvent } from "react-native-webview"; import { parseMessage } from "@/utils/parseMessage"; import { useHandleNavigationActions } from "@/hooks/useHandleNavigationActions"; export function useWebViewMessageHandler() { const handleNavigationActions = useHandleNavigationActions(); const requestOnMessage = (e: WebViewMessageEvent) => { try { const { type, data } = parseMessage(e); handleNavigationActions({ type, data }); } catch (error) { console.error('메시지 파싱 중 오류 발생:', error); } }; return { requestOnMessage }; }이를 각 라우트 파일에서 다음과 같이 사용할 수 있습니다:
export default function MeetingsScreen() { const baseUrl = getWebViewApiUrl(); const { requestOnMessage } = useWebViewMessageHandler(); return <WebView className="flex-1" source={{ uri: `${baseUrl}${ROUTES.MEETINGS}` }} onMessage={requestOnMessage} />; }apps/mobile/app/(route)/settings.tsx (1)
13-15: 통합 테스트 추가 권장WebView 메시지 처리 로직이 변경되었으므로, parseMessage와 handleNavigationActions의 통합 테스트가 필요합니다.
다음과 같은 테스트 케이스 추가를 제안합니다:
// __tests__/integration/WebViewMessageHandling.test.tsx import { renderHook } from '@testing-library/react-hooks'; import { useWebViewMessageHandler } from '@/hooks/useWebViewMessageHandler'; describe('WebView 메시지 처리 통합 테스트', () => { it('올바른 메시지 형식이 전달되면 정상적으로 처리되어야 함', () => { const { result } = renderHook(() => useWebViewMessageHandler()); const mockEvent = { nativeEvent: { data: JSON.stringify({ type: 'TEST_TYPE', data: { test: 'data' } }) } }; expect(() => result.current.requestOnMessage(mockEvent as any)) .not.toThrow(); }); it('잘못된 메시지 형식이 전달되면 에러가 발생해야 함', () => { const { result } = renderHook(() => useWebViewMessageHandler()); const mockEvent = { nativeEvent: { data: 'invalid-json' } }; expect(() => result.current.requestOnMessage(mockEvent as any)) .toThrow(); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/preview.yml(1 hunks)apps/mobile/app/(route)/dashboard.tsx(1 hunks)apps/mobile/app/(route)/meetings.tsx(1 hunks)apps/mobile/app/(route)/seats.tsx(1 hunks)apps/mobile/app/(route)/settings.tsx(1 hunks)apps/web/app/dashboard/_components/DashboardSection.tsx(1 hunks)apps/web/next.config.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/preview.yml
- apps/web/app/dashboard/_components/DashboardSection.tsx
🔇 Additional comments (1)
apps/mobile/app/(route)/dashboard.tsx (1)
13-15: parseMessage 구현 검증 필요parseMessage 유틸리티의 타입 안전성과 구현을 확인해야 합니다.
다음 스크립트를 실행하여 parseMessage 구현을 확인하겠습니다:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/_components/AuthGuard.tsx (1)
36-42: PR 프리뷰 URL 처리 로직 개선이 필요합니다.정규식과 문자열 조작을 통한 현재 구현은 복잡성을 증가시킬 수 있습니다.
다음과 같이 리팩토링하는 것을 제안드립니다:
- const isRootPath = path.endsWith("/") || /\/pr-\d+$/.exec(path); - const targetPath = isRootPath ? `${path}/dashboard`.replace(/\/+/g, "/") : path; + const isRootPath = path.endsWith("/") || path.match(/\/pr-\d+$/); + const normalizedPath = path.replace(/\/+/g, "/"); + const targetPath = isRootPath ? `${normalizedPath}/dashboard` : normalizedPath;변경사항:
exec()대신 더 명확한match()를 사용- 경로 정규화를 별도 단계로 분리
- 조건부 로직을 더 읽기 쉽게 구성
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/_components/AuthGuard.tsx(1 hunks)
🔇 Additional comments (3)
apps/web/app/_components/AuthGuard.tsx (3)
27-28: 경로 추출 및 PR URL 감지 로직이 명확합니다.현재 URL 경로를 추출하고 PR 프리뷰 URL을 감지하는 로직이 간단하고 이해하기 쉽게 구현되어 있습니다.
30-34: 일반 URL 처리 로직이 적절합니다.루트 경로("/")에 대한 처리가 명확하며, 대시보드로의 리다이렉션이 올바르게 구현되어 있습니다.
27-42: 전체 라우팅 로직 검증이 필요합니다.PR 프리뷰 URL이 의도한 대로 작동하는지 확인이 필요합니다.
다음 시나리오들을 검증해주세요:
/pr-123->/pr-123/dashboard/pr-123/->/pr-123/dashboard/pr-123/some-path->/pr-123/some-path✅ Verification successful
PR 프리뷰 URL 라우팅 로직이 정상적으로 구현되었습니다.
요청하신 시나리오들이 모두 올바르게 처리됩니다:
/pr-123→/pr-123/dashboard로 리다이렉트/pr-123/→/pr-123/dashboard로 리다이렉트/pr-123/some-path→ 변경 없이 유지🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # PR 프리뷰 URL 패턴 검증 # 테스트할 URL 패턴들 echo "Testing URL patterns..." echo "1. /pr-123" echo "2. /pr-123/" echo "3. /pr-123/some-path" # 관련 코드 검색 echo -e "\nSearching for related routing logic..." rg -l "router.replace" --type tsLength of output: 480
Script:
#!/bin/bash # Search for PR URL related patterns and routing logic echo "Searching for PR URL patterns..." rg "pr-" -A 3 --type ts echo -e "\nSearching for path/routing related code..." rg "router\.(push|replace)" -A 2 --type ts echo -e "\nSearching for dashboard related redirects..." rg "dashboard" --type ts -A 2Length of output: 4541
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/Gnb/GnbMenu.tsx (1)
36-48: URL 파라미터 처리 로직 개선이 필요합니다.다음과 같은 개선사항을 제안드립니다:
- PR 쿼리 파라미터명을 상수로 분리
- URL 파라미터 타입 안전성 강화
- 잘못된 URL 형식에 대한 예외 처리 추가
+const PR_QUERY_PARAM = 'pr'; const getPrNumber = (): string | null => { if (typeof window === "undefined") return null; - const urlParams = new URLSearchParams(window.location.search); + try { + const urlParams = new URLSearchParams(window.location.search); + return urlParams.get(PR_QUERY_PARAM); + } catch (error) { + console.error('URL 파라미터 파싱 중 오류 발생:', error); + return null; + } - return urlParams.get("pr"); }; const getUrlWithPR = (path: string): string => { const prNumber = getPrNumber(); - return prNumber ? `${path}?pr=${prNumber}` : path; + return prNumber ? `${path}?${PR_QUERY_PARAM}=${prNumber}` : path; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/_components/AuthGuard.tsx(3 hunks)apps/web/components/Gnb/GnbMenu.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/_components/AuthGuard.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (2)
apps/web/components/Gnb/GnbMenu.tsx (2)
50-51: 구현이 적절합니다!PR 파라미터를 유지하면서 네비게이션을 처리하는 로직이 잘 구현되어 있습니다.
74-74: 네비게이션 일관성 검증이 필요합니다.PR 파라미터 유지를 위한
getUrlWithPR적용이 잘 되어있습니다. 다만, 모든 네비게이션 케이스에서 일관성있게 동작하는지 확인이 필요합니다.다음 스크립트로 모든 네비게이션 관련 코드를 검증해보세요:
Also applies to: 99-99
✅ Verification successful
네비게이션 구현이 적절하게 되어 있습니다.
PR 파라미터 처리가 필요한 GnbMenu에서만
getUrlWithPR을 사용하고 있으며, 다른 네비게이션은 의도된 대로 직접 라우팅을 사용하고 있습니다. 플랫폼 간 일관성도useAppRouter를 통해 잘 유지되고 있습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 네비게이션 관련 코드에서 URL 처리 일관성 검증 # Link 컴포넌트 사용 검색 echo "Checking Link component usage..." rg "as={.*Link}" apps/web # href 속성 검색 echo "Checking href attribute usage..." rg "href=" apps/web # push 메서드 사용 검색 echo "Checking router.push usage..." rg "push\(" apps/webLength of output: 1498
Script:
#!/bin/bash # Check getUrlWithPR implementation and usage echo "Checking getUrlWithPR implementation..." ast-grep --pattern 'function getUrlWithPR' -A 10 echo "\nChecking getUrlWithPR usage..." rg "getUrlWithPR" apps/web -A 2 echo "\nChecking useAppRouter implementation..." cat apps/web/app/_hooks/useAppRouter.ts echo "\nChecking Link component imports and usage patterns..." rg "import.*Link" apps/webLength of output: 2457
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/next.config.mjs (1)
1-2: 환경 변수 유효성 검사 추가 필요환경 변수가 존재하지 않을 경우를 대비한 유효성 검사가 필요합니다.
다음과 같이 수정하는 것을 제안드립니다:
-const isPR = process.env.GITHUB_EVENT_NAME === "pull_request"; -const prNumber = process.env.GITHUB_EVENT_NUMBER; +const isPR = process.env.GITHUB_EVENT_NAME === "pull_request" || false; +const prNumber = process.env.GITHUB_EVENT_NUMBER || '';turbo.json (1)
19-21: 환경 변수 선언 포맷팅 개선 필요환경 변수 선언의 일관성을 위해 마지막 항목에도 쉼표를 추가하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
"NODE_ENV", "GITHUB_EVENT_NAME", - "GITHUB_EVENT_NUMBER" + "GITHUB_EVENT_NUMBER",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/preview.yml(2 hunks)apps/web/next.config.mjs(1 hunks)turbo.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/preview.yml
🔇 Additional comments (1)
apps/web/next.config.mjs (1)
8-10: PR 번호를 URL에 노출하는 것에 대한 검토 필요PR 번호를 assetPrefix에 포함시키는 것이 보안상 안전한지 검토가 필요합니다. 외부에서 PR 번호를 통해 프리뷰 환경에 접근할 수 있을 수 있습니다.
또한 이 변경사항이 실제로 프리뷰 URL 문제를 해결하는지 확인이 필요합니다. 다음 스크립트로 검증해보시기 바랍니다:
✅ Verification successful
PR 번호를 URL에 포함하는 것은 안전합니다
현재 구현은 다음과 같은 이유로 보안상 안전합니다:
- CloudFront를 통한 콘텐츠 전송으로 직접적인 접근이 제한됩니다
- AWS 인프라를 통해 접근 제어가 적절히 이루어지고 있습니다
- PR 번호를 URL에 포함하는 것은 프리뷰 환경에서 일반적인 관행입니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the PR preview URL configuration is properly set up in the GitHub Actions workflow # Check if the preview workflow is properly configured to use these environment variables echo "Checking preview workflow configuration..." rg -A 5 "GITHUB_EVENT_NAME|GITHUB_EVENT_NUMBER" .github/workflows/Length of output: 670
Script:
#!/bin/bash # Check preview deployment configuration and security measures # Look for preview URL configuration and access controls echo "Checking preview deployment configuration..." rg -g "*.yml" -g "*.yaml" -A 10 "preview" .github/workflows/ # Check for S3/CloudFront configuration echo -e "\nChecking S3/CloudFront configuration..." rg -g "*.yml" -g "*.yaml" -g "*.ts" -g "*.js" "cloudfront|s3.+bucket" .github/workflows/Length of output: 1814
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/preview.yml (1)
54-57: 빌드 결과 검증 방식을 개선하세요.index.html 파일의 전체 내용을 로그에 출력하는 것은 불필요하게 로그를 길게 만들고, 잠재적으로 민감한 정보를 노출할 수 있습니다.
다음과 같이 파일 크기와 존재 여부만 확인하는 것을 제안합니다:
- name: Check build output run: | - echo "Checking index.html content:" - cat out/index.html + echo "Verifying build output:" + ls -lh out/index.html
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/preview.yml(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/preview.yml
45-45: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🔇 Additional comments (2)
.github/workflows/preview.yml (2)
10-10: PR 이벤트 트리거 설정이 적절합니다!PR이 열리거나, 업데이트되거나, 다시 열릴 때만 워크플로우가 실행되도록 하여 불필요한 빌드와 배포를 방지합니다.
67-71: S3 배포 로깅이 잘 개선되었습니다!S3 동기화 과정과 업로드된 파일 목록을 확인할 수 있도록 하여 배포 과정의 투명성과 디버깅 용이성이 향상되었습니다.
.github/workflows/preview.yml
Outdated
| - name: Check AuthGuard content | ||
| run: | | ||
| echo "AuthGuard content:" | ||
| cat apps/web/app/components/auth/AuthGuard.tsx | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보안: AuthGuard 내용 노출 위험이 있습니다.
AuthGuard.tsx 파일의 내용을 로그에 출력하는 것은 인증 로직이 노출될 수 있어 보안상 위험합니다. 디버깅이 필요한 경우 파일 존재 여부만 확인하거나, 민감하지 않은 정보만 선택적으로 출력하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안합니다:
- - name: Check AuthGuard content
- run: |
- echo "AuthGuard content:"
- cat apps/web/app/components/auth/AuthGuard.tsx
+ - name: Verify AuthGuard file
+ run: |
+ echo "Checking if AuthGuard exists..."
+ test -f apps/web/app/components/auth/AuthGuard.tsx && echo "AuthGuard file exists"📝 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: Check AuthGuard content | |
| run: | | |
| echo "AuthGuard content:" | |
| cat apps/web/app/components/auth/AuthGuard.tsx | |
| - name: Verify AuthGuard file | |
| run: | | |
| echo "Checking if AuthGuard exists..." | |
| test -f apps/web/app/components/auth/AuthGuard.tsx && echo "AuthGuard file exists" |
| run: | | ||
| echo "Current branch: ${{ github.head_ref }}" | ||
| echo "PR number: ${{ github.event.pull_request.number }}" | ||
| echo "Event name: ${{ github.event_name }}" | ||
| pnpm -filter=web build | ||
| env: | ||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보안: github.head_ref 사용 방식을 개선해야 합니다.
GitHub Actions 보안 가이드라인에 따르면, github.head_ref와 같은 신뢰할 수 없는 입력값을 직접 스크립트에서 사용하는 것은 위험할 수 있습니다.
다음과 같이 환경 변수를 통해 전달하도록 수정하는 것을 제안합니다:
- name: Build
run: |
- echo "Current branch: ${{ github.head_ref }}"
+ echo "Current branch: $GITHUB_HEAD_REF"
echo "PR number: ${{ github.event.pull_request.number }}"
echo "Event name: ${{ github.event_name }}"
pnpm -filter=web build
env:
+ GITHUB_HEAD_REF: ${{ github.head_ref }}
GITHUB_EVENT_NAME: ${{ github.event_name }}
GITHUB_EVENT_NUMBER: ${{ 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.
| run: | | |
| echo "Current branch: ${{ github.head_ref }}" | |
| echo "PR number: ${{ github.event.pull_request.number }}" | |
| echo "Event name: ${{ github.event_name }}" | |
| pnpm -filter=web build | |
| env: | |
| GITHUB_EVENT_NAME: ${{ github.event_name }} | |
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} | |
| run: | | |
| echo "Current branch: $GITHUB_HEAD_REF" | |
| echo "PR number: ${{ github.event.pull_request.number }}" | |
| echo "Event name: ${{ github.event_name }}" | |
| pnpm -filter=web build | |
| env: | |
| GITHUB_HEAD_REF: ${{ github.head_ref }} | |
| GITHUB_EVENT_NAME: ${{ github.event_name }} | |
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} |
🧰 Tools
🪛 actionlint (1.7.4)
45-45: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/preview.yml (1)
40-48:⚠️ Potential issue보안: github.head_ref 사용 방식을 개선해야 합니다.
GitHub Actions 보안 가이드라인에 따르면,
github.head_ref와 같은 신뢰할 수 없는 입력값을 직접 스크립트에서 사용하는 것은 위험할 수 있습니다.다음과 같이 환경 변수를 통해 전달하도록 수정하는 것을 제안합니다:
- name: Build run: | - echo "Current branch: ${{ github.head_ref }}" + echo "Current branch: $GITHUB_HEAD_REF" echo "PR number: ${{ github.event.pull_request.number }}" echo "Event name: ${{ github.event_name }}" pnpm -filter=web build env: + GITHUB_HEAD_REF: ${{ github.head_ref }} GITHUB_EVENT_NAME: ${{ github.event_name }} GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }}🧰 Tools
🪛 actionlint (1.7.4)
40-40: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/preview.yml(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/preview.yml
40-40: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🔇 Additional comments (1)
.github/workflows/preview.yml (1)
10-10: PR 트리거 설정이 적절합니다!
opened,synchronize,reopened이벤트에 대한 명시적인 트리거 설정은 워크플로우의 실행 조건을 명확하게 정의하는 좋은 방법입니다.
.github/workflows/preview.yml
Outdated
| run: | | ||
| echo "Syncing files to S3..." | ||
| aws s3 sync ${{ secrets.BUILD_DIRECTORY }} s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --delete | ||
| echo "Files uploaded to S3:" | ||
| aws s3 ls s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --recursive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
보안: S3 파일 목록 노출 제한이 필요합니다.
S3에 업로드된 모든 파일 목록을 로그에 출력하는 것은 민감한 정보가 노출될 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
- name: Deploy to S3 with PR-specific prefix
run: |
echo "Syncing files to S3..."
aws s3 sync ${{ secrets.BUILD_DIRECTORY }} s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --delete
- echo "Files uploaded to S3:"
- aws s3 ls s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --recursive
+ echo "Deployment summary:"
+ aws s3 ls s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --recursive --summarize | grep "Total Objects:"📝 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.
| run: | | |
| echo "Syncing files to S3..." | |
| aws s3 sync ${{ secrets.BUILD_DIRECTORY }} s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --delete | |
| echo "Files uploaded to S3:" | |
| aws s3 ls s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --recursive | |
| run: | | |
| echo "Syncing files to S3..." | |
| aws s3 sync ${{ secrets.BUILD_DIRECTORY }} s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --delete | |
| echo "Deployment summary:" | |
| aws s3 ls s3://${{ secrets.AWS_S3_BUCKET_NAME }}/pr-${{ github.event.pull_request.number }} --recursive --summarize | grep "Total Objects:" |
.github/workflows/preview.yml
Outdated
| - name: Check build output | ||
| run: | | ||
| echo "Checking index.html content:" | ||
| cat out/index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
보안: index.html 내용 노출 제한이 필요합니다.
빌드 결과물인 index.html 파일의 전체 내용을 로그에 출력하는 것은 민감한 정보가 노출될 수 있어 보안상 위험합니다.
다음과 같이 수정하는 것을 제안합니다:
- name: Check build output
run: |
- echo "Checking index.html content:"
- cat out/index.html
+ echo "Verifying build output..."
+ test -f out/index.html && echo "✓ index.html exists"
+ echo "File size: $(stat -f%z out/index.html) bytes"📝 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: Check build output | |
| run: | | |
| echo "Checking index.html content:" | |
| cat out/index.html | |
| - name: Check build output | |
| run: | | |
| echo "Verifying build output..." | |
| test -f out/index.html && echo "✓ index.html exists" | |
| echo "File size: $(stat -f%z out/index.html) bytes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/preview.yml (3)
57-66:⚠️ Potential issue빌드 로깅에서 보안 취약점이 발견되었습니다.
github.head_ref를 직접 사용하는 것은 보안 위험이 있습니다.이전 리뷰 코멘트와 동일한 이슈가 발견되었습니다. 환경 변수를 통해 안전하게 전달하도록 수정해야 합니다.
🧰 Tools
🪛 actionlint (1.7.4)
58-58: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
67-70:⚠️ Potential issueindex.html 내용 노출 제한이 필요합니다.
빌드 결과물의 전체 내용을 로그에 출력하는 것은 보안상 위험합니다.
이전 리뷰 코멘트와 동일한 이슈가 발견되었습니다. 파일 존재 여부와 크기만 확인하도록 수정해야 합니다.
80-84:⚠️ Potential issueS3 파일 목록 노출 제한이 필요합니다.
S3에 업로드된 모든 파일 목록을 로그에 출력하는 것은 보안상 위험합니다.
이전 리뷰 코멘트와 동일한 이슈가 발견되었습니다. 업로드된 파일의 총 개수만 표시하도록 수정해야 합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/preview.yml(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/preview.yml
58-58: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🔇 Additional comments (2)
.github/workflows/preview.yml (2)
10-10: PR 트리거 설정이 적절합니다.
types: [opened, synchronize, reopened]설정으로 PR 이벤트를 명확하게 제한하여 불필요한 워크플로우 실행을 방지합니다.
27-36:⚠️ Potential issue민감한 파일 내용 노출 위험이 있습니다.
파일 내용을 직접 출력하는 것은 보안상 위험합니다.
다음과 같이 수정하는 것을 제안합니다:
- name: Check PR changes run: | - echo "DashboardSection content:" - cat apps/web/app/dashboard/_components/DashboardSection.tsx + echo "DashboardSection 파일 확인:" + test -f apps/web/app/dashboard/_components/DashboardSection.tsx && echo "✓ 파일이 존재합니다" - name: Check AuthGuard content run: | - echo "Looking for AuthGuard.tsx:" - find . -name "AuthGuard.tsx" + echo "AuthGuard.tsx 파일 확인:" + find . -name "AuthGuard.tsx" -exec echo "✓ 파일 위치: {}" \;Likely invalid or redundant comment.
.github/workflows/preview.yml
Outdated
| - name: Check git status | ||
| run: | | ||
| echo "Current branch and commit:" | ||
| git branch | ||
| git rev-parse HEAD | ||
| echo "Last commit message:" | ||
| git log -1 --pretty=%B |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Git 상태 확인 단계를 최적화해야 합니다.
현재 구현은 디버깅에 도움이 되지만, 보안상 민감한 정보가 노출될 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
- name: Check git status
run: |
- echo "Current branch and commit:"
- git branch
- git rev-parse HEAD
- echo "Last commit message:"
- git log -1 --pretty=%B
+ echo "Git 상태 확인 중..."
+ echo "브랜치: ${GITHUB_HEAD_REF}"
+ echo "커밋 해시: ${GITHUB_SHA:0:7}"
+ echo "PR 번호: ${PR_NUMBER}"
+ env:
+ GITHUB_HEAD_REF: ${{ github.head_ref }}
+ GITHUB_SHA: ${{ github.sha }}
+ PR_NUMBER: ${{ 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: Check git status | |
| run: | | |
| echo "Current branch and commit:" | |
| git branch | |
| git rev-parse HEAD | |
| echo "Last commit message:" | |
| git log -1 --pretty=%B | |
| - name: Check git status | |
| run: | | |
| echo "Git 상태 확인 중..." | |
| echo "브랜치: ${GITHUB_HEAD_REF}" | |
| echo "커밋 해시: ${GITHUB_SHA:0:7}" | |
| echo "PR 번호: ${PR_NUMBER}" | |
| env: | |
| GITHUB_HEAD_REF: ${{ github.head_ref }} | |
| GITHUB_SHA: ${{ github.sha }} | |
| PR_NUMBER: ${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/preview.yml(2 hunks)
🔇 Additional comments (3)
.github/workflows/preview.yml (3)
10-10: PR 트리거 타입 추가가 적절합니다.PR 이벤트 타입을 명시적으로 지정하여 워크플로우 실행을 더 정확하게 제어할 수 있게 되었습니다.
18-20: Git checkout 설정이 보안을 강화합니다.특정 SHA를 참조하고 전체 히스토리를 가져오도록 설정하여 보안성이 향상되었습니다.
Line range hint
67-77: Preview URL 댓글의 보안 강화가 필요합니다.이전 리뷰에서 지적된
github.head_ref직접 사용 문제가 여전히 존재합니다.다음과 같이 수정하는 것을 권장합니다:
- name: Add Preview URL as a Comment uses: marocchino/sticky-pull-request-comment@v2 with: message: | ## 🚀 Preview URL - **Branch:** ${{ github.head_ref }} - **Commit:** ${{ github.sha }} + **Branch:** $GITHUB_HEAD_REF + **Commit:** ${GITHUB_SHA:0:7} Preview URL: https://codeit.click?pr=${{ github.event.pull_request.number }} env: + GITHUB_HEAD_REF: ${{ github.head_ref }} + GITHUB_SHA: ${{ github.sha }}
| run: | | ||
| echo "GITHUB_EVENT_NAME=${{ github.event_name }}" >> apps/web/.env.production.local | ||
| echo "GITHUB_EVENT_NUMBER=${{ github.event.pull_request.number }}" >> apps/web/.env.production.local | ||
| pnpm -filter=web build | ||
| env: | ||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
환경 변수 처리 방식의 보안 강화가 필요합니다.
GitHub Actions의 컨텍스트 변수를 직접 사용하는 것은 보안상 위험할 수 있습니다.
다음과 같이 수정하는 것을 권장합니다:
run: |
- echo "GITHUB_EVENT_NAME=${{ github.event_name }}" >> apps/web/.env.production.local
- echo "GITHUB_EVENT_NUMBER=${{ github.event.pull_request.number }}" >> apps/web/.env.production.local
+ echo "GITHUB_EVENT_NAME=$GITHUB_EVENT_NAME" >> apps/web/.env.production.local
+ echo "GITHUB_EVENT_NUMBER=$GITHUB_EVENT_NUMBER" >> apps/web/.env.production.local
pnpm -filter=web build
env:
GITHUB_EVENT_NAME: ${{ github.event_name }}
GITHUB_EVENT_NUMBER: ${{ 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.
| run: | | |
| echo "GITHUB_EVENT_NAME=${{ github.event_name }}" >> apps/web/.env.production.local | |
| echo "GITHUB_EVENT_NUMBER=${{ github.event.pull_request.number }}" >> apps/web/.env.production.local | |
| pnpm -filter=web build | |
| env: | |
| GITHUB_EVENT_NAME: ${{ github.event_name }} | |
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} | |
| run: | | |
| echo "GITHUB_EVENT_NAME=$GITHUB_EVENT_NAME" >> apps/web/.env.production.local | |
| echo "GITHUB_EVENT_NUMBER=$GITHUB_EVENT_NUMBER" >> apps/web/.env.production.local | |
| pnpm -filter=web build | |
| env: | |
| GITHUB_EVENT_NAME: ${{ github.event_name }} | |
| GITHUB_EVENT_NUMBER: ${{ github.event.pull_request.number }} |
🚀 작업 내용
🚨 관련 이슈 (이슈 번호)
✅ 체크리스트
Summary by CodeRabbit
새로운 기능
parseMessage추가UI 변경
CI/CD
리디렉션 개선
환경 변수 추가
GITHUB_EVENT_NAME및GITHUB_EVENT_NUMBER환경 변수 추가