Skip to content

fix: keep generated browser CI least-privileged - #1759

Merged
kang-heewon merged 2 commits into
trunkfrom
fix/1675-browser-workflow-permissions
Aug 5, 2026
Merged

fix: keep generated browser CI least-privileged#1759
kang-heewon merged 2 commits into
trunkfrom
fix/1675-browser-workflow-permissions

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Aug 5, 2026

Copy link
Copy Markdown
Member

Outcome

Generated browser-test workflows now grant the GitHub token only read access to repository contents and disable credential persistence in every checkout step. The generated-app smoke parses the workflow and rejects top-level or job-level permission escalation, persistent checkout credentials, and mixed-case checkout references.

Fixes #1675

Verification

  • pnpm exec vitest run scripts/tests/create-croco-app-generated-smoke.spec.ts --config vitest.config.ts — 40 passed
  • pnpm --filter create-croco-app test — 140 passed
  • pnpm --filter create-croco-app lint, typecheck, and build — passed
  • pnpm package-manifests:check — 115 checked, 0 modified
  • pnpm public-api:check — 115 package snapshots matched
  • pnpm check — 24/25 passed, 1 not applicable
  • Pre-push workspace gates — 234/234 test tasks and 233/233 typecheck tasks passed

Review gates

  • Correctness and regression — PASS. Production and admin generated outputs are parsed as YAML; tests prove exact read-only permissions, all checkout steps disable persisted credentials, and workflow/job escalation and mixed-case checkout bypasses fail.
  • API, security, compatibility, and release — PASS. No public API, runtime dependency, secret, deployment permission, or user-authored workflow changed; package manifests and public API snapshots match, and a patch changeset covers create-croco-app.
  • Maintainability and minimality — PASS. The change is limited to the shared browser workflow template, its generated-output and smoke policy checks, test-only YAML parsing support, lockfile, and release metadata.
  • Independent adversarial review — PASS after fix. Review found a case-insensitive GitHub checkout reference bypass; matching was normalized, an adversarial regression was added, and re-review found no remaining actionable issue.

Residual risk

None identified within the issue scope.

Summary by CodeRabbit

  • 보안 개선

    • 생성되는 브라우저 테스트 워크플로의 저장소 콘텐츠 권한을 읽기 전용으로 제한했습니다.
    • 체크아웃 단계에서 인증 정보가 저장되지 않도록 기본 설정을 강화했습니다.
    • 생성된 워크플로가 최소 권한 보안 정책을 준수하는지 자동으로 검증합니다.
  • 품질 개선

    • 운영형 앱과 관리자 콘솔 생성 과정에서 브라우저 워크플로 보안 설정을 점검합니다.
    • 잘못된 권한 상승이나 인증 정보 저장 설정을 자동으로 감지합니다.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kang-heewon, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 38 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 74a4bc9e-dcf6-4dd2-bead-a5a1061038b8

📥 Commits

Reviewing files that changed from the base of the PR and between 1a11468 and c2bad2a.

📒 Files selected for processing (3)
  • packages/create-croco-app/src/tests/e2e-generation.spec.ts
  • scripts/create-croco-app-generated-smoke.mts
  • scripts/tests/create-croco-app-generated-smoke.spec.ts
📝 Walkthrough

Walkthrough

생성된 브라우저 테스트 워크플로에 contents: read 권한과 persist-credentials: false 설정을 추가했습니다. 생성 테스트와 스모크 검증은 YAML 정책과 checkout 설정을 검사합니다.

Changes

브라우저 워크플로 최소 권한

Layer / File(s) Summary
워크플로 권한 및 checkout 설정
packages/create-croco-app/templates/spa-be-split/.github/workflows/browser-tests.yml, .changeset/quiet-pandas-read.md
워크플로가 저장소 콘텐츠를 읽기만 하도록 설정했습니다. 네 개의 actions/checkout 단계에서 자격 증명 저장을 비활성화했습니다.
생성 앱 워크플로 테스트
packages/create-croco-app/package.json, packages/create-croco-app/src/tests/e2e-generation.spec.ts, packages/create-croco-app/src/tests/templates-build.spec.ts
yaml을 추가하고 생성된 워크플로의 권한, checkout 단계, 고정 버전, 자격 증명 설정을 검증합니다. 운영형 앱과 관리자 콘솔 생성 테스트에 검증을 연결했습니다.
스모크 정책 검증 연결
scripts/create-croco-app-generated-smoke.mts
스모크 검증에 browserWorkflowPolicy를 추가했습니다. YAML 정책 검증 함수가 전역 및 작업별 권한과 checkout 설정을 검사합니다.
정책 검증 테스트
scripts/tests/create-croco-app-generated-smoke.spec.ts
최소 권한 구성은 허용하고 추가 권한, 작업별 권한 상승, 자격 증명 영속화, 잘못된 checkout 식별자는 거부합니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GeneratedApp as 생성 앱
  participant RunValidation as runValidation
  participant PolicyValidator as assertGeneratedBrowserWorkflowLeastPrivilege
  GeneratedApp->>RunValidation: browserWorkflowPolicy 경로 전달
  RunValidation->>PolicyValidator: browser-tests.yml 검증 호출
  PolicyValidator->>GeneratedApp: YAML 정책 읽기
  PolicyValidator-->>RunValidation: 권한 및 checkout 설정 검증 결과
Loading

Possibly related PRs

  • croco-dev/framework#1566: 동일한 브라우저 테스트 워크플로와 관련된 변경이며, 본 PR은 해당 워크플로의 권한과 checkout 자격 증명을 강화합니다.
  • croco-dev/framework#1116: create-croco-app 생성 앱의 보안 검증을 수정하지만, 검증 대상은 HTTP 미들웨어입니다.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 생성된 브라우저 CI 워크플로를 최소 권한으로 유지하는 핵심 변경을 명확하고 간결하게 설명합니다.
Linked Issues check ✅ Passed 템플릿 워크플로, 정책 검증, YAML 파싱 테스트, 스모크 검증이 이슈 #1675의 모든 코딩 요구사항을 충족합니다.
Out of Scope Changes check ✅ Passed 변경된 의존성, 테스트, 스모크 검증, 변경 세트는 모두 생성된 브라우저 워크플로의 최소 권한 보장 범위에 해당합니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1675-browser-workflow-permissions

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 3694-3712: Reject mixed-case actions/checkout references by first
locating checkout steps case-insensitively, then validating the original uses
value begins with the exact case-sensitive actions/checkout@ prefix. Apply this
in scripts/create-croco-app-generated-smoke.mts lines 3694-3712 and
packages/create-croco-app/src/tests/e2e-generation.spec.ts lines 361-372. In
scripts/tests/create-croco-app-generated-smoke.spec.ts lines 97-100, add a
mixed-case fixture retaining persist-credentials: false and assert that the
casing error is rejected.
🪄 Autofix

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 35f1fbe3-f37e-468d-a7fc-bc7e8c24248b

📥 Commits

Reviewing files that changed from the base of the PR and between 20c8b4a and 1a11468.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • .changeset/quiet-pandas-read.md
  • packages/create-croco-app/package.json
  • packages/create-croco-app/src/tests/e2e-generation.spec.ts
  • packages/create-croco-app/src/tests/templates-build.spec.ts
  • packages/create-croco-app/templates/spa-be-split/.github/workflows/browser-tests.yml
  • scripts/create-croco-app-generated-smoke.mts
  • scripts/tests/create-croco-app-generated-smoke.spec.ts

Comment thread scripts/create-croco-app-generated-smoke.mts
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

📊 Benchmark Results

✅ All benchmarks passed

Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 36.9μs 30.0ms 8.2μs +351.0% -
CrocoApp lambdaHandler (10 controllers) 2.1ms 50.0ms 258.4μs +706.4% -
Lambda cold-start simulation 1.6ms 80.0ms 418.1μs +292.7% -
Lambda cold-start with headers 1.5ms 80.0ms 369.7μs +303.0% -
Lambda cold-start with binary body 1.5ms 80.0ms 339.1μs +333.1% -
Lambda cold-start with query params 1.5ms 80.0ms 301.3μs +381.7% -
Lambda cold-start with authorizer context 1.4ms 80.0ms 299.8μs +376.2% -
Lambda cold-start realistic scenario 1.4ms 80.0ms 299.2μs +377.5% -
EventBusConfig.start (10 handlers) 1.9μs 10.0ms 1.4μs +34.9% -
EventPublisher.publishNow single event 2.1μs 2.0ms 1.7μs +25.5% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.1μs -11.2% -
Container.get singleton (cold) 68.3μs 5.0ms 70.3μs -2.7% -
Container.register × 50 components 3.4ms 10.0ms 3.2ms +3.9% -
Container.validate (50 components) 3.7ms 20.0ms 3.4ms +10.1% -
Container.get singleton (warm) 1.7μs 500.0μs 1.6μs +1.8% -
TelemetryRuntime.init (lambda preset) 2.5μs 200.0ms 1.1ms -99.8% -
lambdaPreset config creation 1.4μs 2.0ms 1.4μs +1.4% -

Updated: 2026-08-05T16:05:01.035Z · Commit: feefb23

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai resolve

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved and changes approved.

@kang-heewon
kang-heewon merged commit cfe0d14 into trunk Aug 5, 2026
12 checks passed
@kang-heewon
kang-heewon deleted the fix/1675-browser-workflow-permissions branch August 5, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[create-croco-app] Keep generated browser-test workflows least-privileged

1 participant