Conversation
WalkthroughGitHub Actions에 SECRET_KEY 디코딩으로 키스토어 파일을 생성하는 단계가 추가되고, 기존 인라인 Maestro 실행/녹화 로직이 제거되어 Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI (GitHub Actions)
participant Script as Run Script (.github/scripts/run_maestro_suite.sh)
participant Device as Android Device (adb)
participant Maestro as Maestro CLI
CI->>Script: 호출 (suite_path, suite_name, APP_ID)
Script->>Device: adb shell screenrecord 시작 (백그라운드)
Script->>Maestro: maestro test 실행 (APP_ID 환경변수)
Maestro-->>Script: 테스트 종료 (exit code)
Script->>Device: adb shell screenrecord 종료
Script->>Device: adb pull /sdcard/.../video.mp4 -> recordings/
Script->>CI: 종료 코드 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/maestro.yml (1)
27-27: 셸 변수를 따옴표로 감싸주세요.
$SECRET_KEY를 따옴표 없이 사용하면 셸 word splitting 및 globbing 문제가 발생할 수 있습니다. 특수 문자가 포함된 경우 예기치 않은 동작이 발생할 수 있으므로 따옴표로 감싸는 것이 안전합니다.🔧 권장 수정 사항
- echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks + echo "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/maestro.yml at line 27, Wrap the shell variable SECRET_KEY in quotes when passing it to base64 to prevent word splitting and globbing; update the pipeline that currently echoes SECRET_KEY into base64 (the command containing SECRET_KEY and base64 -di redirecting to team_kap_android.jks) to use a quoted variable (or use printf '%s' with the quoted variable) so special characters and whitespace are preserved safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/maestro.yml:
- Line 27: Wrap the shell variable SECRET_KEY in quotes when passing it to
base64 to prevent word splitting and globbing; update the pipeline that
currently echoes SECRET_KEY into base64 (the command containing SECRET_KEY and
base64 -di redirecting to team_kap_android.jks) to use a quoted variable (or use
printf '%s' with the quoted variable) so special characters and whitespace are
preserved safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0713423c-ca64-4eae-936a-b35725f7dc6f
📒 Files selected for processing (1)
.github/workflows/maestro.yml
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| env: | ||
| SECRET_KEY: ${{ secrets.SECRET_KEY }} | ||
| run: | | ||
| echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks |
There was a problem hiding this comment.
[Major] 환경 변수 미인용(Unquoted Variable) — 잠재적 파싱 오류
$SECRET_KEY가 큰따옴표로 감싸지지 않아, 값 안에 공백·개행·글로브 문자가 포함될 경우 셸이 단어 분리(word splitting)를 수행해 base64에 전달되는 입력이 손상됩니다.
Base64 인코딩된 문자열은 보통 공백을 포함하지 않지만, 멀티라인 인코딩이나 accidentally trailing newline이 있을 때 조용히 실패할 수 있습니다.
또한 echo는 일부 셸에서 -e 플래그 없이도 백슬래시 이스케이프를 해석하므로, printf '%s' 사용이 더 안전합니다.
| echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks | |
| printf '%s' "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks |
| for file in $(find . -name 'team_kap_android.jks') | ||
| do | ||
| echo $file | ||
| done |
There was a problem hiding this comment.
[Minor] 불필요하게 복잡한 파일 존재 검증 로직
for file in $(find . -name 'team_kap_android.jks') 구문은 다음 두 가지 문제가 있습니다:
- 저장소 루트부터 전체 트리를 탐색하므로, 대형 프로젝트에서는 불필요한 I/O가 발생합니다.
$(find ...)커맨드 치환은 결과에 공백이 포함된 경로가 있을 경우 단어 분리로 인해 의도치 않게 동작합니다.
keystore가 정확한 경로에 생성되었는지만 검증하면 충분합니다.
| for file in $(find . -name 'team_kap_android.jks') | |
| do | |
| echo $file | |
| done | |
| ls -la ./koin/team_kap_android.jks |
| with: | ||
| properties: ${{ secrets.PROPERTIES }} | ||
|
|
||
| - name: Update Secrets |
There was a problem hiding this comment.
[Info] 전체적인 접근 방식 — 적절한 CI/CD 패턴
Keystore 파일을 Base64로 인코딩해 GitHub Secrets에 저장하고, CI 실행 시 디코딩하여 사용하는 방식은 Android CI/CD에서 널리 사용되는 표준 패턴입니다. 파일 자체를 레포에 커밋하지 않고 Secrets를 통해 관리한다는 점에서 보안 측면으로도 올바른 선택입니다.
한 가지 추가로 고려할 점은, base64 -di 옵션의 -i(ignore garbage) 플래그가 Linux(GNU coreutils) 에서만 유효하다는 것입니다. GitHub Actions의 ubuntu-* 러너에서는 문제없지만, macOS 러너(macos-*)로 변경할 경우 base64 -D를 사용해야 합니다. 현재 워크플로우가 Linux 러너를 사용한다면 현재 코드는 정상 동작합니다.
bcsd-b-bot
bot
added
ci:passed
and removed
ci:passed
coderabbit:complete
claude:complete
labels
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/run_maestro_suite.sh:
- Line 29: The cleanup command adb shell rm -f "/sdcard/${suite_name}.mp4" can
fail and, with set -euo pipefail enabled, will overwrite the test exit status;
protect this teardown step by making the command non-fatal (append || true) so
failures in the cleanup won’t cause the script to exit with error, mirroring the
protection used for the preceding adb pull command.
- Around line 5-7: The script reads positional variables suite_path, suite_name,
and app_id directly under set -u which causes unbound variable errors if
arguments are missing and lacks usage help; add an explicit argument check at
start (validate $1,$2,$3), print a usage message and exit non-zero when missing,
and then assign suite_path, suite_name, app_id. For cleanup, make the tear-down
commands consistent: ensure the kill, adb pull, and adb shell rm -f commands
tolerate failures without masking the test result by appending || true (or
capture their exit codes but not override $test_exit_code), and ensure the
script exits with $test_exit_code at the end so cleanup failures don't clobber
the real test outcome; reference the existing variables and commands:
suite_path, suite_name, app_id, set -u, kill, adb pull, adb shell rm -f, and
$test_exit_code.
In @.github/workflows/maestro.yml:
- Around line 23-31: Update the "Update Secrets" step to quote the SECRET_KEY
usage like the other secrets: replace unquoted echo $SECRET_KEY with a quoted
echo "$SECRET_KEY" (reference: SECRET_KEY usage inside the Update Secrets run
block) so the value is safely passed to base64 -di and behavior matches the
handling of $FIREBASE and $GOOGLE_CREDENTIALS elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 110130e7-f07e-4115-bdaa-fecaed8464ab
📒 Files selected for processing (2)
.github/scripts/run_maestro_suite.sh.github/workflows/maestro.yml
| suite_path="$1" | ||
| suite_name="$2" | ||
| app_id="$3" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# run_maestro_suite.sh 호출 지점과 인자 전달 형태를 점검합니다.
rg -nP '\brun_maestro_suite\.sh\b' .github/workflows .github/scripts
nl -ba .github/scripts/run_maestro_suite.sh | sed -n '1,20p'Repository: BCSDLab/KOIN_ANDROID
Length of output: 374
🏁 Script executed:
cat .github/scripts/run_maestro_suite.shRepository: BCSDLab/KOIN_ANDROID
Length of output: 738
인자 검증과 cleanup 단계 오류 처리를 추가하세요.
-
인자 검증 누락: 현재
set -u활성 상태에서 인자 검증 없이 Line 5-7에서$1,$2,$3를 직접 참조합니다. 인자 누락 시unbound variable오류로 즉시 종료되며, 사용법을 명확히 하지 않습니다. -
Cleanup 오류 처리 불일치: Line 28-29의
kill과adb pull은|| true로 오류를 무시하지만, Line 30의adb shell rm -f는 오류 처리가 없습니다. 이 명령 실패 시 전체 스크립트가 실패하여$test_exit_code값을 무시하고 cleanup 오류를 반환하게 됩니다.
🔧 제안 패치
#!/usr/bin/env bash
set -euo pipefail
+if [ "$#" -ne 3 ]; then
+ echo "Usage: $0 <suite_path> <suite_name> <app_id>" >&2
+ exit 1
+fi
+
suite_path="$1"
suite_name="$2"
app_id="$3" sleep 2
adb pull "/sdcard/${suite_name}.mp4" "recordings/${suite_name}.mp4" || true
-adb shell rm -f "/sdcard/${suite_name}.mp4"
+adb shell rm -f "/sdcard/${suite_name}.mp4" || true
exit "$test_exit_code"🧰 Tools
🪛 GitHub Actions: Android Maestro UI Tests
[error] 6-6: Script error: line 6: $2: unbound variable. The command 'bash .github/scripts/run_maestro_suite.sh' failed with exit code 1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/run_maestro_suite.sh around lines 5 - 7, The script reads
positional variables suite_path, suite_name, and app_id directly under set -u
which causes unbound variable errors if arguments are missing and lacks usage
help; add an explicit argument check at start (validate $1,$2,$3), print a usage
message and exit non-zero when missing, and then assign suite_path, suite_name,
app_id. For cleanup, make the tear-down commands consistent: ensure the kill,
adb pull, and adb shell rm -f commands tolerate failures without masking the
test result by appending || true (or capture their exit codes but not override
$test_exit_code), and ensure the script exits with $test_exit_code at the end so
cleanup failures don't clobber the real test outcome; reference the existing
variables and commands: suite_path, suite_name, app_id, set -u, kill, adb pull,
adb shell rm -f, and $test_exit_code.
| sleep 2 | ||
|
|
||
| adb pull "/sdcard/${suite_name}.mp4" "recordings/${suite_name}.mp4" || true | ||
| adb shell rm -f "/sdcard/${suite_name}.mp4" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# set -e 활성화 여부와 unguarded cleanup 명령 존재 여부를 확인합니다.
nl -ba .github/scripts/run_maestro_suite.sh | sed -n '1,40p'Repository: BCSDLab/KOIN_ANDROID
Length of output: 104
🏁 Script executed:
cat -n .github/scripts/run_maestro_suite.shRepository: BCSDLab/KOIN_ANDROID
Length of output: 955
정리 단계가 테스트 결과(exit code)를 덮어쓸 수 있습니다.
set -euo pipefail이 활성화되어 있는데, 라인 29의 정리 명령이 보호되지 않아 있습니다. 이 명령이 실패하면 Maestro 테스트 결과와 무관하게 스크립트가 실패 상태로 종료됩니다. 라인 28의 adb pull 명령처럼 정리 작업은 || true로 보호되어야 합니다.
🔧 제안 패치
-adb shell rm -f "/sdcard/${suite_name}.mp4"
+adb shell rm -f "/sdcard/${suite_name}.mp4" || true📝 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.
| adb shell rm -f "/sdcard/${suite_name}.mp4" | |
| adb shell rm -f "/sdcard/${suite_name}.mp4" || true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/run_maestro_suite.sh at line 29, The cleanup command adb
shell rm -f "/sdcard/${suite_name}.mp4" can fail and, with set -euo pipefail
enabled, will overwrite the test exit status; protect this teardown step by
making the command non-fatal (append || true) so failures in the cleanup won’t
cause the script to exit with error, mirroring the protection used for the
preceding adb pull command.
| - name: Update Secrets | ||
| env: | ||
| SECRET_KEY: ${{ secrets.SECRET_KEY }} | ||
| run: | | ||
| echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks | ||
| for file in $(find . -name 'team_kap_android.jks') | ||
| do | ||
| echo $file | ||
| done |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# workflow 내 비인용 secret 디코딩 패턴을 점검합니다.
rg -nP 'echo \$[A-Z_]+\s*\|\s*base64\s+-d' .github/workflows/maestro.yml
nl -ba .github/workflows/maestro.yml | sed -n '23,35p'Repository: BCSDLab/KOIN_ANDROID
Length of output: 194
🏁 Script executed:
# Read the maestro.yml file to see full context around the flagged lines
cat -n .github/workflows/maestro.yml | sed -n '20,110p'Repository: BCSDLab/KOIN_ANDROID
Length of output: 3977
🏁 Script executed:
# Verify the exact behavior of echo vs printf with special characters in secrets
# Test case: simulate a base64-encoded secret and see if unquoted echo preserves it
python3 << 'EOF'
import subprocess
import base64
# Create a test secret (simulating a keystore base64)
test_keystore = b"\x00\x01\x02\x03\xff\xfe\xfd\xfc" # binary data
secret_b64 = base64.b64encode(test_keystore).decode('ascii')
print(f"Test secret (base64): {secret_b64}")
# Test 1: unquoted echo (vulnerable)
cmd_unquoted = f'bash -c \'echo ${secret_b64}\' | base64 -d | od -An -tx1'
result_unquoted = subprocess.run(cmd_unquoted, shell=True, capture_output=True, text=True)
print(f"\nUnquoted echo result: {result_unquoted.stdout.strip()}")
# Test 2: quoted printf (safe)
cmd_quoted = f'bash -c \'printf "%s" "${secret_b64}\' | base64 -d | od -An -tx1'
result_quoted = subprocess.run(cmd_quoted, shell=True, capture_output=True, text=True)
print(f"Quoted printf result: {result_quoted.stdout.strip()}")
# Show if they match
print(f"\nResults match: {result_unquoted.stdout.strip() == result_quoted.stdout.strip()}")
EOFRepository: BCSDLab/KOIN_ANDROID
Length of output: 166
🏁 Script executed:
# Verify if there are multiple variations or alternate lines we might have missed
cat -n .github/workflows/maestro.yml | grep -E 'base64|run_maestro'Repository: BCSDLab/KOIN_ANDROID
Length of output: 506
🏁 Script executed:
# Check if there are other maestro.yml files in the repo
fd -n maestro.ymlRepository: BCSDLab/KOIN_ANDROID
Length of output: 294
Line 27의 SECRET_KEY 변수를 인용하세요.
Line 27의 echo $SECRET_KEY는 비인용 상태이지만, 같은 파일의 Line 38-39에서는 동일한 패턴의 다른 시크릿들($FIREBASE, $GOOGLE_CREDENTIALS)이 모두 인용되어 있습니다. 일관성과 안정성을 위해 SECRET_KEY도 동일하게 처리하세요.
🔧 제안 패치
- echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks
+ echo "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks📝 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: Update Secrets | |
| env: | |
| SECRET_KEY: ${{ secrets.SECRET_KEY }} | |
| run: | | |
| echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks | |
| for file in $(find . -name 'team_kap_android.jks') | |
| do | |
| echo $file | |
| done | |
| - name: Update Secrets | |
| env: | |
| SECRET_KEY: ${{ secrets.SECRET_KEY }} | |
| run: | | |
| echo "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks | |
| for file in $(find . -name 'team_kap_android.jks') | |
| do | |
| echo $file | |
| done |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/maestro.yml around lines 23 - 31, Update the "Update
Secrets" step to quote the SECRET_KEY usage like the other secrets: replace
unquoted echo $SECRET_KEY with a quoted echo "$SECRET_KEY" (reference:
SECRET_KEY usage inside the Update Secrets run block) so the value is safely
passed to base64 -di and behavior matches the handling of $FIREBASE and
$GOOGLE_CREDENTIALS elsewhere.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/maestro.yml (1)
27-27:⚠️ Potential issue | 🟡 MinorSECRET_KEY 디코딩 명령은 반드시 인용/고정 출력으로 처리하세요.
Line 27의
echo $SECRET_KEY는 셸 확장으로 값이 변형될 수 있습니다.printf "%s" "$SECRET_KEY"로 바꿔야 디코딩 안정성이 보장됩니다.🔧 제안 패치
- echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks + printf "%s" "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks#!/bin/bash set -euo pipefail # Line 23-31 구간에서 SECRET_KEY 디코딩 구문 확인 nl -ba .github/workflows/maestro.yml | sed -n '23,31p' # 비인용 echo 패턴 탐지 (매치되면 수정 필요) rg -nP 'echo\s+\$SECRET_KEY\s*\|\s*base64\s+-d[i]?\b' .github/workflows/maestro.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/maestro.yml at line 27, The secret decoding command currently uses an unquoted shell expansion ("echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks") which can mangle special characters; replace that use of echo with a safe, fixed-output invocation such as printf "%s" "$SECRET_KEY" piped to base64 -di and writing to ./koin/team_kap_android.jks so the SECRET_KEY value is not subject to shell word-splitting or interpretation; update the workflow step that contains the echo $SECRET_KEY pipe to use printf "%s" "$SECRET_KEY" | base64 -di > ./koin/team_kap_android.jks (keep the redirection target unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/maestro.yml:
- Line 27: The secret decoding command currently uses an unquoted shell
expansion ("echo $SECRET_KEY | base64 -di > ./koin/team_kap_android.jks") which
can mangle special characters; replace that use of echo with a safe,
fixed-output invocation such as printf "%s" "$SECRET_KEY" piped to base64 -di
and writing to ./koin/team_kap_android.jks so the SECRET_KEY value is not
subject to shell word-splitting or interpretation; update the workflow step that
contains the echo $SECRET_KEY pipe to use printf "%s" "$SECRET_KEY" | base64 -di
> ./koin/team_kap_android.jks (keep the redirection target unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1208877a-340a-4042-9be9-4ba9cb6caa84
📒 Files selected for processing (1)
.github/workflows/maestro.yml
1 participant
PR 개요
PR 체크리스트
작업사항
작업사항의 상세한 설명
논의 사항
스크린샷
추가내용
Summary by CodeRabbit