security(deploy-pages): declare minimal secret contract - #901
security(deploy-pages): declare minimal secret contract#901seonghobae wants to merge 10 commits into
Conversation
|
Warning Review limit reached
Next review available in: 51 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCloudflare Pages 재사용 워크플로가 ChangesCloudflare Pages 시크릿 계약
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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
| declared = set( | ||
| re.findall(r"^ ([A-Z][A-Z0-9_]+):$", call_contract, re.MULTILINE) | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
유효한 YAML 변형도 차단하십시오.
현재 검사는 EXTRA_SECRET: # 설명 형태의 추가 시크릿을 수집하지 않습니다. 또한 secrets: inherit 또는 secrets: inherit # 설명은 금지 검사를 통과합니다. 이 경우 테스트는 통과하지만 명시적 매핑 계약은 위반됩니다.
tests/test_deploy_pages_secret_contract.py#L85-L87: 콜론 뒤의 공백·주석과 관계없이 6칸 들여쓰기의 시크릿 키를 수집하십시오.tests/test_deploy_pages_secret_contract.py#L111-L114: 공백, 선택적 따옴표, 후행 주석을 허용하는 줄 단위 정규식으로inherit를 거부하십시오.
수정 예시
- re.findall(r"^ ([A-Z][A-Z0-9_]+):$", call_contract, re.MULTILINE)
+ re.findall(
+ r"^ ([A-Z][A-Z0-9_]+):(?:[ \t].*)?$",
+ call_contract,
+ re.MULTILINE,
+ )
...
- assert "secrets: inherit" not in example
+ assert not re.search(
+ r"""(?m)^[ \t]*secrets[ \t]*:[ \t]*(?:inherit|["']inherit["'])[ \t]*(?:#.*)?$""",
+ example,
+ )📝 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.
| declared = set( | |
| re.findall(r"^ ([A-Z][A-Z0-9_]+):$", call_contract, re.MULTILINE) | |
| ) | |
| declared = set( | |
| re.findall( | |
| r"^ ([A-Z][A-Z0-9_]+):(?:[ \t].*)?$", | |
| call_contract, | |
| re.MULTILINE, | |
| ) | |
| ) |
| declared = set( | |
| re.findall(r"^ ([A-Z][A-Z0-9_]+):$", call_contract, re.MULTILINE) | |
| ) | |
| for example in examples: | |
| assert not re.search( | |
| r"""(?m)^[ \t]*secrets[ \t]*:[ \t]*(?:inherit|["']inherit["'])[ \t]*(?:#.*)?$""", | |
| example, | |
| ) | |
| assert "CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}" in example | |
| assert "CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}" in example |
📍 Affects 1 file
tests/test_deploy_pages_secret_contract.py#L85-L87(this comment)tests/test_deploy_pages_secret_contract.py#L111-L114
🤖 Prompt for 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.
In `@tests/test_deploy_pages_secret_contract.py` around lines 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.
1 participant
Summary
secrets: inheritguidance with an explicit reusable-workflow interface for exactlyCLOUDFLARE_API_TOKENandCLOUDFLARE_ACCOUNT_IDcontents: read, caller checkout, action pins, and deployment behaviorRCA 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, the authoritative examples map both names explicitly, and the called workflow itself references only those two Cloudflare names.A current central/organization search found no product caller requiring migration. This must be revalidated immediately before merge; any new thin caller must be repaired in its owning repository rather than widening this reusable contract.
Validation
workflow_call.secrets).github/workflows/deploy-pages.ymlgit diff --check1e87aa7ecad8e5aeb33d5483039199fe081f45d8Exact current status
1e87aa7ecad8e5aeb33d5483039199fe081f45d8;COMMENTEDreview is predecessor-head evidence, not a formal approval.Protected-main operational acceptance remains required: one mapped-secrets deployment and one missing-mapping negative control without value disclosure.
Fixes #900
Summary by CodeRabbit
변경 사항
문서
테스트