Proposal: Pin GitHub Actions to commit SHAs for supply-chain security#115
Proposal: Pin GitHub Actions to commit SHAs for supply-chain security#115ysknsid25 wants to merge 10 commits into
Conversation
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
|
@ysknsid25 is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR hardens supply-chain security by pinning GitHub Actions to immutable commit SHAs and adding pnpm v11 workspace governance. It pins 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
… comments Signed-off-by: ysknsid25 <kengo071225@gmail.com>
dosubot
Bot
added
size:XS
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens the repository's CI/CD supply chain by pinning GitHub Actions to commit SHAs, adding a pinact workflow to enforce SHA pinning, configuring Dependabot for monthly Action updates, and tightening pnpm install behavior with a minimumReleaseAge cooldown and an explicit build allowlist. It also bumps the packageManager to pnpm@11.3.0.
Changes:
- Pin all
actions/checkout,actions/setup-node, andpnpm/action-setupreferences to commit SHAs with version comments. - Add
.github/workflows/pinact.ymland.github/dependabot.ymlto enforce/maintain SHA pinning. - Update
pnpm-workspace.yamlwithminimumReleaseAge,minimumReleaseAgeExclude, andallowBuilds; bumppackageManagertopnpm@11.3.0.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Pin every actions/checkout@v6 to SHA de0fac2… (v6.0.2). |
.github/workflows/publish.yml |
Same SHA-pin update for checkout actions. |
.github/actions/environment/action.yml |
Pin pnpm/action-setup and actions/setup-node to SHAs. |
.github/workflows/pinact.yml |
New workflow that runs pinact to enforce SHA pinning on PRs touching .github/**. |
.github/dependabot.yml |
New Dependabot config for monthly grouped GitHub Actions updates. |
pnpm-workspace.yaml |
Add release-age cooldown, valibot exclusion, and build allowlist. |
package.json |
Bump packageManager from pnpm@10.24.0 to pnpm@11.3.0. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ns to SHA Signed-off-by: ysknsid25 <kengo071225@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ci.yml:
- Around line 15-16: Every checkout step using the actions/checkout action (the
steps named "Checkout repository" that reference uses:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) must set
persist-credentials: false to avoid writing GITHUB_TOKEN into .git/config;
update each checkout step in the workflow to include persist-credentials: false
under that step so the token is not persisted (apply the same change to every
recurring checkout step in the file).
In @.github/workflows/pinact.yml:
- Line 19: The inline comment "# v1.1.0" next to the pinned action SHA is
inaccurate because the pinned SHA (cf51507d80d4d6522a07348e3d58790290eaf0b6)
does not match the upstream v1.1.0 commit; update the uses entry for
suzuki-shunsuke/pinact-action to either the actual v1.1.0 commit SHA
(bd070edaef6bf653de2b9ef59fe6bde498152c17) or remove/replace the trailing "#
v1.1.0" comment so the comment and the pinned SHA are consistent (look for the
uses line referencing pinact-action and the trailing comment to change).
In @.github/workflows/publish.yml:
- Around line 21-22: Update every actions/checkout step in the workflow that
currently uses "uses: actions/checkout@..." to set persist-credentials: false
(e.g., replace or add the checkout steps where actions/checkout is used in the
publish jobs) so the GITHUB_TOKEN is not persisted; ensure each checkout step in
the publish jobs also keeps any existing id-token: write setting but adds the
top-level persist-credentials: false key for security.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c92a4855-8e66-4087-9fef-27a6a528a15c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/actions/environment/action.yml.github/dependabot.yml.github/workflows/ci.yml.github/workflows/pinact.yml.github/workflows/publish.ymlpackage.jsonpnpm-workspace.yaml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:16">
P2: Set `persist-credentials: false` on CI checkout steps. The default leaves the `GITHUB_TOKEN` written to `.git/config`, where it can leak into uploaded artifacts. These jobs only build/test and never push, so persistence is unnecessary. The `pinact.yml` workflow already sets this correctly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
ysknsid25
left a comment
ysknsid25
left a comment
There was a problem hiding this comment.
@fabian-hiller
I have an implementation question 🙏
There was a problem hiding this comment.
With this update, all versions written in current workflow files will be SHA-fixed. However, future files may still contain descriptions like @v6. As a countermeasure, how about introducing a static analysis tool like pinact that checks for non-SHA-fixed descriptions?
There was a problem hiding this comment.
I don't know much about pinact. So I don't have a strong opinion. I try to keep things simple but if there is a good reasons to use such a tools it should be ok. I will do a decision in my final review before merging it.
There was a problem hiding this comment.
Understood. Let me elaborate on the reasons.
- Currently, everything is pinned using SHA.
- When a new CI is added (for example, like this), if it's written as
@v6, only that part becomes vulnerable. - Pinact is a static analysis tool used to prevent this.
There's no particular reason for sticking with Pinact. It's simply the one my company uses.
ysknsid25
force-pushed
the
chore/pin-github-actions-sha
branch
from
5421d09 to
cea9f33
Compare
|
We will keep this as a draft until #128 is merged 🙏 |
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
ysknsid25
force-pushed
the
chore/pin-github-actions-sha
branch
from
e078d81 to
df5e9a9
Compare
|
#128 is merged |
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
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 `@pnpm-workspace.yaml`:
- Around line 21-53: The overrides in pnpm-workspace.yaml are changing majors
for specific packages (see entries 'serialize-javascript@6' -> '7.0.5', 'tar@4'
-> '7.5.11' and 'tar@6' -> '7.5.11', and 'undici@5' -> '6.24.0'); update these
overrides so they do not switch major versions: either choose a patched version
within the original major (e.g., replace 'serialize-javascript@6' with a 6.x
patched release, replace the 'tar@4' and 'tar@6' targets with appropriate 4.x
and 6.x patched releases, and replace 'undici@5' with a 5.x patched release) or
remove the cross-major override and instead enforce the fix via lockfile/CI
checks; ensure the changed entries are made in the pnpm-workspace.yaml override
map and run CI/lockfile verification to confirm resolution stability.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0f5ff176-3637-4a6c-997f-f644c0c58060
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/actions/environment/action.ymlpnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/environment/action.yml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@pnpm-workspace.yaml`:
- Around line 21-53: The overrides in pnpm-workspace.yaml are changing majors
for specific packages (see entries 'serialize-javascript@6' -> '7.0.5', 'tar@4'
-> '7.5.11' and 'tar@6' -> '7.5.11', and 'undici@5' -> '6.24.0'); update these
overrides so they do not switch major versions: either choose a patched version
within the original major (e.g., replace 'serialize-javascript@6' with a 6.x
patched release, replace the 'tar@4' and 'tar@6' targets with appropriate 4.x
and 6.x patched releases, and replace 'undici@5' with a 5.x patched release) or
remove the cross-major override and instead enforce the fix via lockfile/CI
checks; ensure the changed entries are made in the pnpm-workspace.yaml override
map and run CI/lockfile verification to confirm resolution stability.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0f5ff176-3637-4a6c-997f-f644c0c58060
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/actions/environment/action.ymlpnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/environment/action.yml
🛑 Comments failed to post (1)
pnpm-workspace.yaml (1)
21-53:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail python - <<'PY' import re, urllib.parse, urllib.request from pathlib import Path text = Path("pnpm-workspace.yaml").read_text(encoding="utf-8") # Match lines like: 'pkg@X': 'Y.Z.W' pattern = re.compile(r"^\s*'([^'@]+(?:/[^'@]+)?@(\d+))':\s*'([^']+)'", re.M) print("== Parsed overrides ==") rows = [] for m in pattern.finditer(text): lhs_full = m.group(1) # e.g. tar@4 lhs_major = int(m.group(2)) # e.g. 4 rhs_ver = m.group(3) # e.g. 7.5.11 pkg = lhs_full.rsplit("@", 1)[0] rhs_major_match = re.match(r"^(\d+)\.", rhs_ver) rhs_major = int(rhs_major_match.group(1)) if rhs_major_match else None rows.append((pkg, lhs_major, rhs_ver, rhs_major)) print(f"- {pkg}@{lhs_major} -> {rhs_ver}") print("\n== Major mismatch check ==") mismatch = [r for r in rows if r[3] is not None and r[1] != r[3]] if mismatch: for pkg, lmaj, rver, rmaj in mismatch: print(f"MAJOR_MISMATCH: {pkg}@{lmaj} -> {rver} (target major {rmaj})") else: print("No cross-major mappings found.") print("\n== npm version existence check ==") for pkg, _, rver, _ in rows: encoded_pkg = urllib.parse.quote(pkg, safe="@") url = f"https://registry.npmjs.org/{encoded_pkg}/{rver}" try: with urllib.request.urlopen(url, timeout=10) as resp: ok = (200 <= resp.status < 300) print(f"EXISTS: {pkg}@{rver}" if ok else f"MISSING: {pkg}@{rver}") except Exception: print(f"MISSING_OR_UNREACHABLE: {pkg}@{rver}") PYRepository: open-circle/formisch
Length of output: 2048
Cross-major
pnpm-workspace.yamloverrides detected
pnpm-workspace.yamlcontains override targets that switch majors, which can break resolution/runtime expectations:
serialize-javascript@6→7.0.5tar@4→7.5.11,tar@6→7.5.11undici@5→6.24.0The
lodash@4 -> 4.18.0case is same-major (not inherently cross-major). The suggested npm-registry “existence” check didn’t return usable results in this environment (all lookups were unreachable), so the install/runtime risk needs to be addressed via pnpm resolution/lockfile impact checks and CI verification rather than registry reachability.🤖 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 `@pnpm-workspace.yaml` around lines 21 - 53, The overrides in pnpm-workspace.yaml are changing majors for specific packages (see entries 'serialize-javascript@6' -> '7.0.5', 'tar@4' -> '7.5.11' and 'tar@6' -> '7.5.11', and 'undici@5' -> '6.24.0'); update these overrides so they do not switch major versions: either choose a patched version within the original major (e.g., replace 'serialize-javascript@6' with a 6.x patched release, replace the 'tar@4' and 'tar@6' targets with appropriate 4.x and 6.x patched releases, and replace 'undici@5' with a 5.x patched release) or remove the cross-major override and instead enforce the fix via lockfile/CI checks; ensure the changed entries are made in the pnpm-workspace.yaml override map and run CI/lockfile verification to confirm resolution stability.
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
resolves: #116
Summary by cubic
Pin GitHub Actions to specific commit SHAs and add a PR check to enforce pins. Tighten
pnpmworkspace security with a 7-day minimum release age and monthly Dependabot updates.actions/checkout(v6.0.2/6.0.3),pnpm/action-setup(v4.3.0,standalone: true),actions/setup-node(v4.4.0),OWASP/cve-lite-cli(v1.19.2), andgithub/codeql-action/upload-sarif(v4.36.2) to SHAs across CI, publish, security workflows, and the composite action.pinactworkflow usingsuzuki-shunsuke/pinact-action(v1.1.0) to verify SHA pins on PRs; fix its config to avoid errors..github/dependabot.ymlwith monthly, grouped updates forgithub-actions.pnpm-workspace.yaml: setminimumReleaseAge: 7 days; extendminimumReleaseAgeExcludeto includevalibot,miniflare@4.20260603.0, andwrangler@4.98.0.Written for commit c9658d0. Summary will update on new commits.