ci: GHA workflow security cleanup#703
Open
emptyhammond wants to merge 4 commits into
Open
Conversation
Set persist-credentials: false on every actions/checkout step so the default GITHUB_TOKEN is not left in the local git config after checkout. None of these jobs push or write back through the checked-out clone, so keeping the credential available was unused exposure.
Add a top-level `permissions: {}` to each workflow and grant each job
only the scopes it actually needs:
- check / integration-test: `contents: read` for checkout,
`checks: write` for the JUnit test reporter.
- docs: `contents: read` for checkout, alongside the existing
`deployments: write` and `id-token: write` used by the SDK
upload and AWS OIDC steps.
- features: `contents: read` for the reusable workflow it calls.
This replaces the default token scope (which depends on repo settings
and historically defaults to read/write across the board).
Move workflow-context interpolations (`${{ ... }}`) out of `run:`
shell source and into per-step `env:` bindings. The shell now sees
ordinary environment variables instead of pre-expanded text, so values
sourced from matrix entries or step outputs cannot terminate the
surrounding quotes and inject shell.
- docs.yml: `steps.sdk-upload-prempt.outputs.url-base` (a step
output) becomes $BASE_URL. `github.sha` is dropped in favour of
the $GITHUB_SHA env var GitHub already exports to every step.
- integration-test.yml: `matrix.protocol` and the JSON/msgpack
derivation become $PROTOCOL and $ABLY_PROTOCOL.
Replace tag-based action references with the matching commit SHA, with the original tag preserved as a trailing comment. Tags are mutable and can be repointed by the upstream owner at any time, so pinning to SHAs keeps the running code stable until we deliberately bump it. Pinned in this commit: - actions/checkout (v2) - actions/setup-go (v2) - ably/test-observability-action (v1) - dorny/test-reporter (v1) - ably/sdk-upload-action (v2) - aws-actions/configure-aws-credentials (v1) - ably/features/.github/workflows/sdk-features.yml (main) The pinned SHAs were taken from each repository's current resolution of the tag/branch, so behaviour matches what was running before this commit. Future updates flow through a normal version bump PR.
Contributor
WalkthroughThis PR hardens four GitHub Actions workflows by pinning third-party actions to specific commit SHAs instead of floating version tags. Workflow and job-level permissions are explicitly declared. The integration test workflow refactors environment variable handling and the docs workflow updates build output references. ChangesCI/CD Workflow Action Pinning and Permissions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/features.yml:
- Line 18: Replace the broad "secrets: inherit" entry in the caller workflow
with an explicit mapping for only the secret consumed by the reusable workflow
(the reusable workflow ably/features/.github/workflows/sdk-features.yml declares
on.workflow_call.secrets and uses ABLY_AWS_ACCOUNT_ID_SDK); remove "secrets:
inherit" and add a single secrets mapping that passes ABLY_AWS_ACCOUNT_ID_SDK
from the caller (ABLY_AWS_ACCOUNT_ID_SDK: ${{ secrets.ABLY_AWS_ACCOUNT_ID_SDK
}}), ensuring least-privilege secret propagation to the reusable workflow.
🪄 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: ff4a2fe3-03d7-4274-8d09-d3a44c0a26dd
📒 Files selected for processing (4)
.github/workflows/check.yml.github/workflows/docs.yml.github/workflows/features.yml.github/workflows/integration-test.yml
| uses: ably/features/.github/workflows/sdk-features.yml@6b3fc7a8ede2ebdd7a6325314f3a96c6466f1453 # main | ||
| with: | ||
| repository-name: ably-go | ||
| secrets: inherit |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the reusable workflow definition to identify required secrets
gh api repos/ably/features/contents/.github/workflows/sdk-features.yml --jq '.content' | base64 -d | grep -A 10 'secrets:'Repository: ably/ably-go
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Caller workflow snippet (.github/workflows/features.yml around line 18) ==="
ls -la .github/workflows || true
nl -ba .github/workflows/features.yml | sed -n '1,120p' | sed -n '10,60p'
echo
echo "=== Reusable workflow: extract full on.workflow_call.secrets block ==="
REPO="ably/ably-go"
TARGET="ably/features/contents/.github/workflows/sdk-features.yml"
# Fetch content from GitHub via gh (as in original comment) and decode
CONTENT_B64="$(gh api "repos/${TARGET}" --jq '.content' 2>/dev/null || true)"
if [[ -z "${CONTENT_B64}" ]]; then
echo "Failed to fetch reusable workflow via gh api."
exit 1
fi
WORKFLOW_YML="$(printf '%s' "${CONTENT_B64}" | base64 -d)"
# Print only the secrets block context if possible
echo "${WORKFLOW_YML}" | rg -n "on:\s*$|workflow_call:|secrets:" -n || true
echo
echo "---- secrets: block (from first secrets: to next top-level key) ----"
# Use awk to roughly extract the block starting at "secrets:" with indentation, until indentation decreases.
# This is robust enough for workflow YAML where secrets are under on.workflow_call with consistent indent.
printf '%s' "${WORKFLOW_YML}" | awk '
BEGIN{inblock=0; blockindent=-1}
{
# detect "secrets:" line
if ($0 ~ /^[[:space:]]*secrets:[[:space:]]*$/) {
# compute indent
match($0, /^[[:space:]]*/); ind=RLENGTH;
inblock=1; blockindent=ind;
print "---- START ----";
print FNR ":" $0;
next;
}
if (inblock==1) {
# stop when indentation goes back to secrets indent level or less AND line looks like a new key
match($0, /^[[:space:]]*/); ind=RLENGTH;
if (ind <= blockindent && $0 ~ /^[[:space:]]*[A-Za-z0-9_]+:[[:space:]]*$/) {
print "---- END ----";
inblock=0;
} else {
print FNR ":" $0;
}
}
}
' | sed -n '1,200p'
echo
echo "=== List all secrets with required: true ==="
# Extract secret names under the secrets block with required: true
printf '%s' "${WORKFLOW_YML}" | awk '
BEGIN{in=0}
/^[[:space:]]*secrets:[[:space:]]*$/ {in=1; next}
in==1 {
# stop when leaving secrets block (indent decreases and line is a new key)
match($0, /^[[:space:]]*/); ind=RLENGTH;
if ($0 ~ /^[[:space:]]*[A-Za-z0-9_]+:[[:space:]]*$/ && ind <= 2) {in=0}
# capture secret key lines like " SECRET_NAME:" (indent 4-ish)
if ($0 ~ /^[[:space:]]*([A-Za-z0-9_]+):[[:space:]]*$/) {
key=$0; gsub(/^[[:space:]]+/,"",key); gsub(/:.*$/,"",key);
lastkey=key
}
if ($0 ~ /required:[[:space:]]*true/) {
if (lastkey!="") print lastkey
}
}
' | sort -u
echo
echo "=== List all secrets keys (regardless of required) ==="
printf '%s' "${WORKFLOW_YML}" | awk '
BEGIN{in=0}
/^[[:space:]]*secrets:[[:space:]]*$/ {in=1; next}
in==1 {
# capture secret key lines
if ($0 ~ /^[[:space:]]*([A-Za-z0-9_]+):[[:space:]]*$/) {
key=$0; gsub(/^[[:space:]]+/,"",key); gsub(/:.*$/,"",key);
print key
}
# naive stop condition: leaving the secrets mapping when indent decreases sharply
match($0, /^[[:space:]]*/); ind=RLENGTH;
if ($0 ~ /^[A-Za-z0-9_]+:[[:space:]]*$/ && ind==0) in=0
}
' | sort -uRepository: ably/ably-go
Length of output: 558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Caller workflow: .github/workflows/features.yml (lines 1-120) ==="
cat -n .github/workflows/features.yml | sed -n '1,120p'
echo
echo "=== Reusable workflow: fetch full on.workflow_call.secrets block ==="
TARGET="ably/features/contents/.github/workflows/sdk-features.yml"
CONTENT_B64="$(gh api "repos/${TARGET}" --jq '.content' 2>/dev/null || true)"
if [[ -z "${CONTENT_B64}" ]]; then
echo "Failed to fetch reusable workflow via gh api."
exit 1
fi
WORKFLOW_YML="$(printf '%s' "${CONTENT_B64}" | base64 -d)"
# Print only the workflow_call -> secrets subtree (best-effort, indentation-based)
# First locate "workflow_call:" and then within it locate "secrets:"
echo "${WORKFLOW_YML}" | awk '
BEGIN{in_wc=0; in_secrets=0}
/^[[:space:]]*workflow_call:[[:space:]]*$/ {in_wc=1; wc_indent=match($0,/^[[:space:]]*/); wc_indent=RLENGTH; print; next}
in_wc==1 {
# stop when leaving workflow_call (indent <= wc indent and new top-level key)
match($0,/^[[:space:]]*/); ind=RLENGTH;
if (ind<=wc_indent && $0 ~ /^[[:space:]]*[A-Za-z0-9_]+:[[:space:]]*$/) {in_wc=0}
}
in_wc==1 {
if ($0 ~ /^[[:space:]]*secrets:[[:space:]]*$/) {in_secrets=1; secrets_indent=match($0,/^[[:space:]]*/); secrets_indent=RLENGTH}
if (in_secrets==1) {
print
match($0,/^[[:space:]]*/); ind=RLENGTH;
# stop secrets when indentation returns to secrets' parent level
if (ind<=secrets_indent && $0 ~ /^[[:space:]]*[A-Za-z0-9_]+:[[:space:]]*$/ && $0 !~ /^[[:space:]]*secrets:[[:space:]]*$/) {in_secrets=0}
}
}
'
echo
echo "=== Extract required secrets (required: true) under on.workflow_call.secrets ==="
# Best-effort extraction:
# Track current secret name while inside secrets mapping; print last secret when a line "required: true" appears.
printf '%s' "${WORKFLOW_YML}" | awk '
BEGIN{in_secrets=0; last_secret=""}
/^[[:space:]]*secrets:[[:space:]]*$/ {in_secrets=1; next}
in_secrets==1 {
# stop when leaving secrets mapping: line with indent <= secrets indent and looks like a new key
match($0,/^[[:space:]]*/); ind=RLENGTH;
if ($0 ~ /^[A-Za-z0-9_]+:[[:space:]]*$/) {in_secrets=0; next}
# detect secret key line like " MY_SECRET:"
if (match($0,/^[[:space:]]*([A-Za-z0-9_]+):[[:space:]]*$/, m)) {
last_secret=m[1]
}
# required true
if ($0 ~ /required:[[:space:]]*true/) {
if (last_secret!="") print last_secret
}
}
' | sort -u
echo
echo "=== Done ==="Repository: ably/ably-go
Length of output: 790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Caller workflow snippet ==="
cat -n .github/workflows/features.yml | sed -n '1,80p'
echo
echo "=== Fetch & parse reusable workflow on.workflow_call.secrets ==="
REPO="ably/ably-go"
TARGET="ably/features/contents/.github/workflows/sdk-features.yml"
# Install PyYAML for reliable YAML parsing
python3 - <<'PY' 2>/dev/null || true
PY
pip show pyyaml >/dev/null 2>&1 || pip install -q pyyaml
python3 - <<'PY'
import os, base64, json, subprocess, sys
import yaml
target = "ably/features/contents/.github/workflows/sdk-features.yml"
# Fetch workflow file content via gh api
cmd = ["gh","api",f"repos/ably/features/contents/.github/workflows/sdk-features.yml","--jq",".content"]
try:
b64 = subprocess.check_output(cmd, text=True).strip()
except subprocess.CalledProcessError as e:
print("Failed to fetch via gh api:", e, file=sys.stderr)
sys.exit(1)
if not b64:
print("No content returned from gh api", file=sys.stderr)
sys.exit(1)
yml_text = base64.b64decode(b64).decode("utf-8", errors="replace")
data = yaml.safe_load(yml_text) or {}
wc = (data.get("on") or {}).get("workflow_call") or {}
secrets = wc.get("secrets") or {}
required = []
all_secrets = []
# secrets is typically a mapping: { SECRET_NAME: { required: true/false, ... } }
if isinstance(secrets, dict):
for name, spec in secrets.items():
all_secrets.append(name)
if isinstance(spec, dict) and spec.get("required") is True:
required.append(name)
print("All secrets declared under on.workflow_call.secrets:", sorted(all_secrets))
print("Secrets with required: true:", sorted(required))
PYRepository: ably/ably-go
Length of output: 679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import base64, subprocess, sys
target = "ably/features/contents/.github/workflows/sdk-features.yml"
cmd = ["gh","api",f"repos/ably/features/contents/.github/workflows/sdk-features.yml","--jq",".content"]
b64 = subprocess.check_output(cmd, text=True).strip()
yml = base64.b64decode(b64).decode("utf-8","replace")
print("=== Raw contains ===")
for s in ["workflow_call","secrets:","ABLY_AWS_ACCOUNT_ID_SDK","required: true"]:
print(s, "=>", s in yml)
print("\n=== Extract lines around workflow_call ===")
lines = yml.splitlines()
for i,l in enumerate(lines):
if "workflow_call" in l:
start=max(0,i-5); end=min(len(lines), i+40)
print(f"\n--- lines {start+1}-{end} around {i+1} ---")
for j in range(start,end):
print(f"{j+1:5d}: {lines[j]}")
break
print("\n=== Extract lines around ABLY_AWS_ACCOUNT_ID_SDK ===")
for i,l in enumerate(lines):
if "ABLY_AWS_ACCOUNT_ID_SDK" in l:
start=max(0,i-10); end=min(len(lines), i+40)
print(f"\n--- lines {start+1}-{end} around {i+1} ---")
for j in range(start,end):
print(f"{j+1:5d}: {lines[j]}")
break
PYRepository: ably/ably-go
Length of output: 3423
🌐 Web query:
GitHub reusable workflows secrets: inherit behavior when on.workflow_call.secrets is not declared
💡 Result:
When using the secrets: inherit keyword in a calling workflow, all secrets available to the calling workflow are passed to the reusable workflow [1][2]. If on.workflow_call.secrets is not declared (or if it is declared but does not explicitly list a specific secret), those inherited secrets are still available for use within the reusable workflow [1][2]. Specifically, the behavior works as follows: 1. Inheritance: By specifying secrets: inherit in the jobs.<job_id>.secrets section of the caller workflow, you authorize the reusable workflow to access all repository, environment, and organization-level secrets that the calling workflow itself can access [1][2][3]. 2. Availability: Once inherited, these secrets can be referenced directly within the steps of the reusable workflow (e.g., using ${{ secrets.MY_SECRET }}) [1][2]. You do not need to explicitly declare them in the on.workflow_call.secrets block for them to be usable [1][2]. 3. Scope: Secrets are only passed to the directly called workflow [1][2]. If you have a chain of workflows (A calls B, B calls C), workflow C will only receive secrets from A if they are explicitly passed from A to B and subsequently from B to C [1][2]. This approach simplifies the management of reusable workflows, as it removes the need to define a static list of secrets in the on.workflow_call interface, allowing the reusable workflow to remain agnostic of the specific secrets required by the caller [4][5].
Citations:
- 1: https://github.com/github/docs/blob/main/content/actions/how-tos/reuse-automations/reuse-workflows.md
- 2: https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows
- 3: https://cloudchronicles.blog/blog/Passing-Environment-Secrets-and-Variables-to-Reusable-Workflows-in-GitHub-Actions/
- 4: Support secret inheritence
secrets: inheritfor actions actions/toolkit#1168 - 5: Enhancement: Streamlining secret inheritance to environment in reusable workflows actions/runner#3379
Scope secrets: inherit in .github/workflows/features.yml to the reusable workflow’s required secret(s).
The reusable workflow ably/features/.github/workflows/sdk-features.yml declares and uses only ABLY_AWS_ACCOUNT_ID_SDK (on.workflow_call.secrets). Using secrets: inherit passes all caller-accessible secrets, breaking least-privilege.
Update to explicitly pass only what’s needed:
secrets:
ABLY_AWS_ACCOUNT_ID_SDK: ${{ secrets.ABLY_AWS_ACCOUNT_ID_SDK }}🤖 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 @.github/workflows/features.yml at line 18, Replace the broad "secrets:
inherit" entry in the caller workflow with an explicit mapping for only the
secret consumed by the reusable workflow (the reusable workflow
ably/features/.github/workflows/sdk-features.yml declares
on.workflow_call.secrets and uses ABLY_AWS_ACCOUNT_ID_SDK); remove "secrets:
inherit" and add a single secrets mapping that passes ABLY_AWS_ACCOUNT_ID_SDK
from the caller (ABLY_AWS_ACCOUNT_ID_SDK: ${{ secrets.ABLY_AWS_ACCOUNT_ID_SDK
}}), ensuring least-privilege secret propagation to the reusable workflow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Routine hygiene pass over the GitHub Actions workflows in this repo, addressing findings from a workflow security audit. Changes are split into four commits, one per finding type:
No behavioural changes intended — the workflows run the same checks against the same inputs.
Summary by CodeRabbit