security(deploy-pages): declare minimal secret contract - #901
security(deploy-pages): declare minimal secret contract#901seonghobae wants to merge 11 commits into
Conversation
📝 WalkthroughWalkthroughCloudflare Pages 재사용 워크플로가 두 개의 필수 Cloudflare 시크릿을 명시적으로 받습니다. 배포 입력을 검증한 뒤 배포, 도메인 연결, 요약 단계에 전달합니다. 문서와 계약 테스트가 새 시크릿 계약과 입력 검증을 반영합니다. ChangesCloudflare Pages 배포 계약
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller as 호출 워크플로
participant Workflow as deploy-pages.yml
participant Cloudflare as Cloudflare Pages
participant DomainAPI as 사용자 지정 도메인 API
participant Summary as 작업 요약
Caller->>Workflow: 두 시크릿과 배포 입력 전달
Workflow->>Workflow: 입력 검증 및 출력 생성
Workflow->>Cloudflare: 검증된 프로젝트명과 빌드 경로로 배포
Workflow->>DomainAPI: 검증된 도메인 연결
Workflow->>Summary: 검증된 값으로 결과 기록
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
/oc Refetch and abort without writing unless exact head is still Keep the production change and tests that require exactly the two declared/consumed Cloudflare names and forbid Add a narrow contract assertion that the doctoring records this platform limitation and caller prohibition without weakening the existing no-inherit examples. Keep all current action pins/permissions/deployment behavior unchanged. Re-run the focused test suite and exact-head checks. Do not invent a YAML feature that blocks |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 @.github/workflows/deploy-pages.yml:
- Line 6: Update the comment in the workflow to state that it references only
the two declared values, without claiming GitHub Actions imposes a runtime
restriction; add that approved callers must use explicit secret mapping.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4d826c52-aabf-4b84-b0c5-c183887a2e64
📒 Files selected for processing (5)
.github/workflows/deploy-pages.ymlCHANGELOG.mddocs/doctoring/deploy-pages-secret-contract.mdinfra/cloudflare/README.mdtests/test_deploy_pages_secret_contract.py
|
Exact-head review requested: The named-interface/platform-inheritance boundary, both authoritative explicit-mapping examples, four contract tests, and workflow-header wording are aligned. All 9 exact-head pull-request workflows succeeded, and the prior line-6 thread is resolved/outdated. @cwl-noema-review @opencode-agent Please review this exact head. Do not carry forward evidence from |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
1e87aa7ecad8e5aeb33d5483039199fe081f45d8. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/31319914444/job/93261130198)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/31319914444/job/93261130198)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: deploy-pages.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: deploy-pages.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (2 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (2 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: deploy-pages-secret-contract.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: deploy-pages-secret-contract.md"]
R3 --> V3["docs review"]
Evidence --> S4["Test: test_deploy_pages_secret_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_deploy_pages_secret_contract.py"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed. Findings1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
Failed checks:
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: deploy-pages.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: deploy-pages.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (2 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (2 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: deploy-pages-secret-contract.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: deploy-pages-secret-contract.md"]
R3 --> V3["docs review"]
Evidence --> S4["Test: test_deploy_pages_secret_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_deploy_pages_secret_contract.py"]
R4 --> V4["targeted test run"]
|
There was a problem hiding this comment.
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 `@tests/test_deploy_pages_secret_contract.py`:
- Around line 85-87: Update tests/test_deploy_pages_secret_contract.py lines
85-87 so the declared-secret regex captures six-space-indented keys even when
the colon is followed by whitespace or a comment, including forms such as
EXTRA_SECRET: # 설명. Update lines 111-114 so the inherit check uses a line-based
regex allowing whitespace, optional quotes, and trailing comments, thereby
rejecting both secrets: inherit and secrets: inherit # 설명; both changes belong
to the existing contract test logic.
🪄 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: CHILL
Plan: Pro Plus
Run ID: ecd5fb7e-4a99-4d76-842a-fb0433fd6a3c
📒 Files selected for processing (3)
.github/workflows/deploy-pages.ymldocs/doctoring/deploy-pages-secret-contract.mdtests/test_deploy_pages_secret_contract.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/deploy-pages.yml
- docs/doctoring/deploy-pages-secret-contract.md
1 participant
Summary
secrets: inheritguidance with an explicit reusable-workflow interface for exactlyCLOUDFLARE_API_TOKENandCLOUDFLARE_ACCOUNT_IDcontents: read, caller checkout, immutable action pins, and deployment behavior while keeping both Cloudflare values requiredproject_name,build_dir, andcustom_domainbefore they reach Wrangler, Cloudflare API URLs, or step summaries; only sealed validator outputs cross those sinksRCA and feasibility
The workflow consumed two named secrets but declared no
workflow_call.secretsinterface, making blanket inheritance the documented transport. The least-privilege root-cause repair is to declare the two consumed names and map them explicitly at callers. Removing the guard alone would not constrain exposure; retaining blanket inheritance with prose warnings would not change the trust boundary.This declaration is a named interface and approved-caller policy, not a GitHub runtime allowlist. GitHub still permits same-organization or same-enterprise callers to use
secrets: inherit; CWL policy prohibits that caller form, authoritative examples map both names explicitly, and the called workflow itself references only the two Cloudflare names.Strix then exposed a second trust boundary: caller-controlled Pages inputs were interpolated into the Wrangler command and Cloudflare URL/summary paths. The repair therefore validates all three inputs before use, canonicalizes the build directory under
GITHUB_WORKSPACE, rejects symlink escape, and passes only validator outputs downstream. A later CodeRabbit finding showed that the contract test itself could miss valid YAML spellings such asEXTRA_SECRET: # commentand quoted/whitespacesecrets: inherit; the current exact-head tests cover those variants rather than relying on brittle literal matching.A current central/organization search found no product caller requiring migration at the time of the secret-contract repair. This must be revalidated immediately before protected integration; any new thin caller must be repaired in its owning repository rather than widening this reusable contract.
Test-first evidence
e7825b137cc359f8522c08794854fe9f39606c4fExact current status
e7825b137cc359f8522c08794854fe9f39606c4f6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba; merge decisions must independently resolve the live protected base tipCHANGES_REQUESTEDreview was submitted against predecessor head1e87aa7ecad8e5aeb33d5483039199fe081f45d8because that older head still had the Strix finding; it is historical evidence and is not a current-head approvalProtected-main operational acceptance remains required after integration: one approved caller with explicit secret mappings and bounded canonical deployment inputs, plus a missing-mapping/invalid-input negative control that stops without value disclosure. Do not infer that acceptance from PR checks.
Fixes #900
Summary by CodeRabbit
변경 사항
문서
테스트