Conversation
…card, concurrency guards
There was a problem hiding this comment.
Pull request overview
This PR continues the repo’s security hardening work by reducing unnecessary credential usage in GitHub Actions, tightening release protections, adding provenance attestations, and introducing OpenSSF Scorecard reporting.
Changes:
- Removed private
basecamp-sdkaccess setup (GOPRIVATE/app-token/git config) fromsecurity.ymlandrelease.yml. - Added release workflow concurrency guard, pinned GoReleaser to an exact version, and added build provenance attestation.
- Added a new weekly OpenSSF Scorecard workflow that uploads SARIF results.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/security.yml |
Removes private module token/GOPRIVATE setup from gosec + CodeQL jobs. |
.github/workflows/scorecard.yml |
Adds a Scorecard workflow with SARIF artifact + Security tab upload. |
.github/workflows/release.yml |
Adds release concurrency, pins GoReleaser, adds provenance attestation, and narrows app token repos/permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 476750e0ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Updates ai-labeler to use the fixed shared workflows that parse JSON responses correctly and include pull-requests:read for private repos.
Idempotent script to audit and enforce release environment protection
across all CLI repos. Supports four modes:
audit — print environment existence, reviewers, deployment
policies, and secret scope per repo
apply — create environments, set required reviewers,
deployment tag policies, and admin bypass rules
migrate-secrets — provision secrets from 1Password to env scope,
optionally delete repo-scoped copies
migrate-vars — move variables from repo to env scope
Driven by a repo manifest. Reads back state after every write and
fails on drift.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 1 file (changes from recent commits).
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="scripts/manage-release-env.sh">
<violation number="1" location="scripts/manage-release-env.sh:148">
P2: Function `set_no_admin_bypass` sets `prevent_self_review`, not admin bypass. `prevent_self_review` prevents a deployment *creator* from approving their own run — it does not prevent admins from bypassing environment protection rules. The function name and manifest field (`admin_bypass`) are misleading. Consider renaming to `set_prevent_self_review` for accuracy, or documenting why this is the intended proxy.</violation>
<violation number="2" location="scripts/manage-release-env.sh:273">
P1: Sequential PUT calls to the same environment endpoint overwrite each other's fields. The GitHub Environments API uses PUT (full replacement), so `set_reviewers` sets reviewers, then `set_deployment_policy` resets reviewers while setting the branch policy, then `set_no_admin_bypass` resets both while setting `prevent_self_review`. The environment can never converge to the desired state.
Consolidate all three settings into a single PUT call that sends `reviewers`, `deployment_branch_policy`, and `prevent_self_review` together.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="scripts/manage-release-env.sh">
<violation number="1" location="scripts/manage-release-env.sh:293">
P3: The refactored `apply_repo` now uses inline API calls, leaving `create_env`, `set_deployment_policy`, `set_reviewers`, and `set_no_admin_bypass` as dead code with no remaining callers. Consider removing them to keep the script easy to follow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove private SDK access (GOPRIVATE, app-token, git config) from all 7 jobs in test.yml — the SDK is now public - Add explicit minimal permissions to security job in release.yml - Remove dead functions from manage-release-env.sh (create_env, set_deployment_policy, set_reviewers, set_no_admin_bypass) - Fix reviewer name formatting and add team identity verification to audit
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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="scripts/manage-release-env.sh">
<violation number="1" location="scripts/manage-release-env.sh:121">
P2: Substring `grep` can false-pass on partial slug matches. For example, expected slug `sip` would match `sip-extended`. Use a word-boundary or exact-element match for this security-audit check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…review
These are two separate GitHub environment fields:
- can_admins_bypass: whether admins can skip protection rules
- prevent_self_review: whether reviewers can approve their own deploys
The script was only setting/checking prevent_self_review. Now sets
can_admins_bypass: false in the PUT payload, and checks it correctly
in both audit and apply verification.
Also fixes jq `// true` gotcha: jq's alternative operator treats
`false` as falsy, so `false // true` returns `true`. Use
`if has("can_admins_bypass") then .can_admins_bypass else true end`
instead.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use exact jq element match instead of grep substring for reviewer slug verification (avoids false positive on partial matches like sip vs sip-extended) - Add reviewer identity check to post-apply verification - Add continue-on-error to upload-sarif step in scorecard workflow (graceful degradation when Advanced Security isn't available)
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
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="scripts/manage-release-env.sh">
<violation number="1" location="scripts/manage-release-env.sh:121">
P2: Use `jq --arg` instead of shell string interpolation to pass `$expected_slug` into the jq filter. If the slug ever contains `"`, `\`, or other jq-special characters, the current concatenation will break the expression. This is a minor robustness issue—but particularly worth fixing in a security-hardening PR.</violation>
<violation number="2" location="scripts/manage-release-env.sh:300">
P2: Same shell-interpolation-in-jq issue as in `audit_repo`. Use `--arg slug "$expected_slug"` and reference `$slug` in the filter.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Verify correct team | ||
| local expected_slug | ||
| expected_slug=$(echo "$reviewer" | cut -d/ -f2) | ||
| if echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then |
There was a problem hiding this comment.
P2: Same shell-interpolation-in-jq issue as in audit_repo. Use --arg slug "$expected_slug" and reference $slug in the filter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/manage-release-env.sh, line 300:
<comment>Same shell-interpolation-in-jq issue as in `audit_repo`. Use `--arg slug "$expected_slug"` and reference `$slug` in the filter.</comment>
<file context>
@@ -294,7 +294,15 @@ apply_repo() {
+ # Verify correct team
+ local expected_slug
+ expected_slug=$(echo "$reviewer" | cut -d/ -f2)
+ if echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then
+ ok "Reviewers verified ($expected_slug)"
+ else
</file context>
| if echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then | |
| if echo "$env_json" | jq -e --arg slug "$expected_slug" '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == $slug)' >/dev/null 2>&1; then |
| # Verify expected team is a reviewer (exact match, not substring) | ||
| local expected_slug | ||
| expected_slug=$(echo "$reviewer" | cut -d/ -f2) | ||
| if ! echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then |
There was a problem hiding this comment.
P2: Use jq --arg instead of shell string interpolation to pass $expected_slug into the jq filter. If the slug ever contains ", \, or other jq-special characters, the current concatenation will break the expression. This is a minor robustness issue—but particularly worth fixing in a security-hardening PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/manage-release-env.sh, line 121:
<comment>Use `jq --arg` instead of shell string interpolation to pass `$expected_slug` into the jq filter. If the slug ever contains `"`, `\`, or other jq-special characters, the current concatenation will break the expression. This is a minor robustness issue—but particularly worth fixing in a security-hardening PR.</comment>
<file context>
@@ -115,10 +115,10 @@ audit_repo() {
local expected_slug
expected_slug=$(echo "$reviewer" | cut -d/ -f2)
- if ! echo "$reviewer_names" | grep -q "$expected_slug"; then
+ if ! echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then
fail "Expected reviewer '$reviewer' not found (have: $reviewer_names)"
drift=1
</file context>
| if ! echo "$env_json" | jq -e '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == "'"$expected_slug"'")' >/dev/null 2>&1; then | |
| if ! echo "$env_json" | jq -e --arg slug "$expected_slug" '[.protection_rules // [] | .[] | select(.type == "required_reviewers")] | .[0].reviewers // [] | [.[].reviewer | .slug // .login] | any(. == $slug)' >/dev/null 2>&1; then |
Summary
concurrencygroupactions/attest-build-provenancev2.14.1instead of floating~> v2permission-contents: writeto Homebrew tap token; addpermission-contents,permission-pull-requests,permission-issuesto skills tokenTest plan
actionlintor GitHub's workflow editor)Summary by cubic
Hardened CI and release workflows and added a release environment management script to enforce protections, migrate secrets/vars, and block admin bypass with a single-PUT apply. Also removed stale private SDK access, tightened job permissions, added build provenance and weekly Scorecards, and made SARIF uploads resilient when Advanced Security is unavailable.
New Features
actions/attest-build-provenance(addsattestations: write).ossf/scorecard-action.scripts/manage-release-env.shto audit/apply env protections and migrate secrets/vars; uses exact reviewer slug matching with post-apply identity checks, reads back state, enforcescan_admins_bypasscorrectly, fails on drift, and handles repos without existing rules.Refactors
basecamp-sdkaccess everywhere, including all 7 jobs intest.yml(dropGOPRIVATE, app token, git config).concurrencygroup to prevent overlapping runs.goreleaser/goreleaser-actiontov2.14.1.release.ymlsecurity job; scoped Homebrew and skills tokens.basecamp/.githubAI labeler workflows to a fixed SHA with JSON parsing fix and PR read perms for private repos.continue-on-errorto tolerate repos without Advanced Security.Written for commit 7af61eb. Summary will update on new commits.