fix: harden credentialed shared actions - #176
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request adds a reusable CLI documentation publishing workflow and hardens existing workflows and composite actions. External actions and release tools use pinned revisions. Checkout credentials, permissions, shell handling, artifact validation, and conditional publishing are updated. ChangesCLI documentation publishing
Action and workflow hardening
Release action hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SourceRepository
participant GenerateJob
participant DocumentationRepository
participant ArtifactStorage
participant PublishJob
SourceRepository->>GenerateJob: run CLI documentation generator
GenerateJob->>DocumentationRepository: write generated Markdown
GenerateJob->>ArtifactStorage: upload validated artifact
PublishJob->>ArtifactStorage: download artifact
PublishJob->>DocumentationRepository: revalidate and replace output directory
PublishJob->>DocumentationRepository: rebase, commit, and push changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
.github/workflows/docs-cli-next.yml (1)
84-87: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse committed integrity data for formatter dependencies.
These commands resolve package content from the registry during each run and do not use committed integrity data. Use a lockfile with
npm ci, or verify the expected SHA-512 checksums before extraction and execution.🤖 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/docs-cli-next.yml around lines 84 - 87, Update the formatter dependency setup in the workflow to use committed integrity data instead of resolving unverified packages at runtime: replace the npm pack/install flow involving ory-prettier-styles and global prettier with a lockfile-driven npm ci installation, or verify the expected SHA-512 checksums before extracting and executing the packages.Source: Linters/SAST tools
🤖 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/docs-cli-next.yml:
- Around line 171-185: Update the “Push generated docs” step to handle a
non-fast-forward push by rebasing the existing local commit onto the latest
DOCS_BRANCH and retrying the push. Keep the normal push path unchanged, and
abort safely without force-pushing if the rebase encounters conflicts.
In @.github/workflows/test.yml:
- Around line 31-37: Replace the hard-coded files array in the workflow
validation step with discovery of all YAML workflow files under
.github/workflows and all action.yml/action.yaml manifests, including
labels.yml. Pass the complete discovered file set to the immutable-dependency
check so every workflow and composite-action manifest is scanned.
In `@newsletter/action.yml`:
- Around line 39-41: Add the requested curl timeout options to the download
command in the newsletter action: use a 15-second connection timeout, 120-second
maximum transfer time, and 300-second maximum retry duration, while preserving
the existing retry count and other flags.
- Around line 53-58: Update the SSH setup around ssh-keyscan and the global
core.sshCommand configuration to validate github.com against GitHub’s published
host key or a pinned fingerprint before writing known_hosts. Configure
core.sshCommand with StrictHostKeyChecking=yes and a job-local
UserKnownHostsFile, while preserving the existing key and ssh-agent setup.
- Around line 20-28: Update the README usage example for the newsletter action
to declare the required caller permission, adding contents: read under
permissions. Keep the existing example configuration and checkout behavior
unchanged.
- Around line 49-58: Update the SSH setup run block to store the key,
known_hosts, and related files under unique paths in $RUNNER_TEMP, and configure
core.sshCommand locally rather than globally. Remove ssh-agent startup and
ssh-add unless the socket and PID are explicitly passed to all required steps.
Add a separate cleanup step with if: ${{ always() }} that removes the temporary
key and SSH files and stops any started agent.
In `@releaser/action.yml`:
- Around line 127-135: Remove blanket “|| true” suppression from the publication
Git commands in releaser/action.yml lines 127-135, allowing stash restoration,
checkout master, pull/rebase, and push failures to stop the action; handle only
the expected no-change commit result explicitly. Apply the same failure-handling
correction to the push flow at releaser/action.yml lines 163-170 before pushing
HEAD:master.
- Around line 149-155: Update the pre-release check in the release workflow to
inspect the triggering release ref `${GITHUB_REF_NAME}` rather than relying on
`git describe --tags` from the worktree switched to master. Preserve the
existing skip message and exit behavior for pre-release refs, while allowing
documentation publishing for regular releases.
---
Nitpick comments:
In @.github/workflows/docs-cli-next.yml:
- Around line 84-87: Update the formatter dependency setup in the workflow to
use committed integrity data instead of resolving unverified packages at
runtime: replace the npm pack/install flow involving ory-prettier-styles and
global prettier with a lockfile-driven npm ci installation, or verify the
expected SHA-512 checksums before extracting and executing the packages.
🪄 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: 9ffeadab-2337-4181-be92-656061495830
📒 Files selected for processing (6)
.github/workflows/docs-cli-next.yml.github/workflows/test.ymlcheckout/action.ymlnewsletter/action.ymlnewsletter/slack-notify/action.ymlreleaser/action.yml
| git stash | ||
| git pull --rebase origin "$DOCS_BRANCH" | ||
| git stash apply | ||
| git add -- "$OUTPUT_DIR" | ||
| git commit -m "autogen(docs): generate cli docs" | ||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||
| - name: Push generated docs | ||
| if: ${{ steps.commit.outputs.changed == 'true' }} | ||
| working-directory: docs | ||
| env: | ||
| DOCS_BRANCH: ${{ inputs.docs-branch }} | ||
| DOCS_TOKEN: ${{ secrets.token }} | ||
| run: | | ||
| git push "https://x-access-token:${DOCS_TOKEN}@github.com/ory/docs.git" \ | ||
| "HEAD:${DOCS_BRANCH}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry after a non-fast-forward push failure.
The rebase occurs before the local commit exists. If two runs start from the same docs-branch tip, one push succeeds and the other push fails. Rebase the committed change onto the latest remote branch and retry the push. Fail safely if the rebase conflicts.
Proposed fix
- git push "https://x-access-token:${DOCS_TOKEN}`@github.com/ory/docs.git`" \
- "HEAD:${DOCS_BRANCH}"
+ for attempt in 1 2 3; do
+ if git push "https://x-access-token:${DOCS_TOKEN}`@github.com/ory/docs.git`" \
+ "HEAD:${DOCS_BRANCH}"; then
+ exit 0
+ fi
+
+ git fetch origin "$DOCS_BRANCH"
+ git rebase "origin/${DOCS_BRANCH}"
+ done
+
+ exit 1📝 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.
| git stash | |
| git pull --rebase origin "$DOCS_BRANCH" | |
| git stash apply | |
| git add -- "$OUTPUT_DIR" | |
| git commit -m "autogen(docs): generate cli docs" | |
| echo "changed=true" >> "$GITHUB_OUTPUT" | |
| - name: Push generated docs | |
| if: ${{ steps.commit.outputs.changed == 'true' }} | |
| working-directory: docs | |
| env: | |
| DOCS_BRANCH: ${{ inputs.docs-branch }} | |
| DOCS_TOKEN: ${{ secrets.token }} | |
| run: | | |
| git push "https://x-access-token:${DOCS_TOKEN}@github.com/ory/docs.git" \ | |
| "HEAD:${DOCS_BRANCH}" | |
| git stash | |
| git pull --rebase origin "$DOCS_BRANCH" | |
| git stash apply | |
| git add -- "$OUTPUT_DIR" | |
| git commit -m "autogen(docs): generate cli docs" | |
| echo "changed=true" >> "$GITHUB_OUTPUT" | |
| - name: Push generated docs | |
| if: ${{ steps.commit.outputs.changed == 'true' }} | |
| working-directory: docs | |
| env: | |
| DOCS_BRANCH: ${{ inputs.docs-branch }} | |
| DOCS_TOKEN: ${{ secrets.token }} | |
| run: | | |
| for attempt in 1 2 3; do | |
| if git push "https://x-access-token:${DOCS_TOKEN}`@github.com/ory/docs.git`" \ | |
| "HEAD:${DOCS_BRANCH}"; then | |
| exit 0 | |
| fi | |
| git fetch origin "$DOCS_BRANCH" | |
| git rebase "origin/${DOCS_BRANCH}" | |
| done | |
| exit 1 |
🤖 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/docs-cli-next.yml around lines 171 - 185, Update the “Push
generated docs” step to handle a non-fast-forward push by rebasing the existing
local commit onto the latest DOCS_BRANCH and retrying the push. Keep the normal
push path unchanged, and abort safely without force-pushing if the rebase
encounters conflicts.
| files=( | ||
| checkout/action.yml | ||
| releaser/action.yml | ||
| newsletter/action.yml | ||
| newsletter/slack-notify/action.yml | ||
| .github/workflows/docs-cli-next.yml | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scan all workflow and composite-action manifests.
Line 31 uses a fixed file list. The check does not scan .github/workflows/labels.yml, which contains mutable actions/checkout@v2 and ory/label-sync-action@v0 references. The immutable-dependency check can pass while mutable references remain in the repository.
Discover all workflow YAML files and action.yml or action.yaml manifests before running the validation.
Proposed fix
- files=(
- checkout/action.yml
- releaser/action.yml
- newsletter/action.yml
- newsletter/slack-notify/action.yml
- .github/workflows/docs-cli-next.yml
- )
+ mapfile -d '' -t files < <(
+ find .github/workflows -type f \
+ \( -name '*.yml' -o -name '*.yaml' \) -print0
+ find . -path ./.git -prune -o -type f \
+ \( -name 'action.yml' -o -name 'action.yaml' \) -print0
+ )📝 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.
| files=( | |
| checkout/action.yml | |
| releaser/action.yml | |
| newsletter/action.yml | |
| newsletter/slack-notify/action.yml | |
| .github/workflows/docs-cli-next.yml | |
| ) | |
| mapfile -d '' -t files < <( | |
| find .github/workflows -type f \ | |
| \( -name '*.yml' -o -name '*.yaml' \) -print0 | |
| find . -path ./.git -prune -o -type f \ | |
| \( -name 'action.yml' -o -name 'action.yaml' \) -print0 | |
| ) |
🤖 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/test.yml around lines 31 - 37, Replace the hard-coded
files array in the workflow validation step with discovery of all YAML workflow
files under .github/workflows and all action.yml/action.yaml manifests,
including labels.yml. Pass the complete discovered file set to the
immutable-dependency check so every workflow and composite-action manifest is
scanned.
| required: false | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - uses: ory/ci/checkout@master | ||
| - run: | | ||
| git fetch origin +refs/tags/*:refs/tags/* | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| with: | ||
| fetch-depth: 0 | ||
| persist-credentials: false |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'newsletter|ssh_key:|mailchimp_api_key:|permissions:|contents:' \
--glob '*.yml' --glob '*.yaml' .Repository: ory/ci
Length of output: 9083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- newsletter action references ---'
rg -n -C 10 \
'(uses:.*newsletter|ory/ci/.*/newsletter|newsletter/action\.yml|ssh_key:)' \
--glob '*.yml' --glob '*.yaml' --glob '*.yaml.tmpl' --glob '*.yml.tmpl' .
printf '%s\n' '--- workflow permission blocks ---'
rg -n -C 8 \
'(^|[[:space:]])permissions:|contents:' \
--glob '*.yml' --glob '*.yaml' --glob '*.yaml.tmpl' --glob '*.yml.tmpl' . || true
printf '%s\n' '--- all tracked workflow/action files ---'
git ls-files | rg '(^|/)(\.github/workflows|action\.yml$|action\.yaml$)'Repository: ory/ci
Length of output: 2099
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
tracked = pathlib.Path(".git").joinpath("index")
files = []
import subprocess
files = subprocess.check_output(["git", "ls-files", "-z"]).decode().split("\0")
newsletter_refs = []
permission_refs = []
for name in files:
if not name:
continue
try:
text = pathlib.Path(name).read_text(errors="replace")
except OSError:
continue
if re.search(r"newsletter", text, re.I):
for number, line in enumerate(text.splitlines(), 1):
if re.search(r"newsletter", line, re.I):
newsletter_refs.append((name, number, line.strip()))
if re.search(r"(^|\s)permissions:|contents:", text):
for number, line in enumerate(text.splitlines(), 1):
if re.search(r"(^|\s)permissions:|contents:", line):
permission_refs.append((name, number, line.strip()))
print("NEWSLETTER_REFERENCES")
for item in newsletter_refs:
print(f"{item[0]}:{item[1]}:{item[2]}")
print("PERMISSION_REFERENCES")
for item in permission_refs:
print(f"{item[0]}:{item[1]}:{item[2]}")
PY
printf '%s\n' 'NEWSLETTER_ACTION'
cat -n newsletter/action.ymlRepository: ory/ci
Length of output: 5242
🌐 Web query:
GitHub Actions actions/checkout token input default GITHUB_TOKEN contents read permission documentation
💡 Result:
The actions/checkout action uses the GITHUB_TOKEN by default (via the token input) to fetch the repository [1][2]. To ensure proper functionality, it is recommended to explicitly set the contents permission to read [1][3]. When using the actions/checkout action, you should configure your workflow or job permissions as follows: permissions: contents: read If you do not explicitly define a permissions block, the GITHUB_TOKEN permissions depend on your repository or organization settings [4]. Following the principle of least privilege by setting contents: read ensures that the checkout action has the minimum access required to clone your repository [1][4][5].
Citations:
- 1: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 2: https://github.com/actions/checkout
- 3: https://github.com/marketplace/actions/checkout
- 4: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
- 5: https://github.com/github/docs/blob/main/content/actions/tutorials/authenticate-with-github_token.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- .github/workflows/test.yml ---'
cat -n .github/workflows/test.yml | sed -n '1,60p'
printf '%s\n' '--- README newsletter usage ---'
cat -n README.md | sed -n '28,58p'
printf '%s\n' '--- newsletter action call sites ---'
rg -n -C 8 \
'uses:.*(newsletter|ory/ci)' \
--glob '*.yml' --glob '*.yaml' --glob 'README*' .Repository: ory/ci
Length of output: 17713
Document the required caller permission.
The in-repository caller grants contents: read, but the README example does not specify this requirement. External callers without repository read access can fail during checkout before ssh_key is configured. Add permissions: contents: read to the usage documentation.
🤖 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 `@newsletter/action.yml` around lines 20 - 28, Update the README usage example
for the newsletter action to declare the required caller permission, adding
contents: read under permissions. Keep the existing example configuration and
checkout behavior unchanged.
| curl --fail --silent --show-error --location --retry 7 \ | ||
| --output "${workdir}/${archive}" \ | ||
| "https://github.com/ory/cli/releases/download/v${version}/${archive}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="newsletter/action.yml"
printf '%s\n' '--- relevant action definition ---'
sed -n '1,90p' "$file"
printf '%s\n' '--- curl invocations and timeout settings ---'
rg -n -C 3 'curl|connect-timeout|max-time|retry-max-time|retry' -- "$file" newsletter .github 2>/dev/null || true
printf '%s\n' '--- curl version and option behavior ---'
curl --version | head -n 2
curl --help all | rg -n -- '--connect-timeout|--max-time|--retry-max-time|--retry'Repository: ory/ci
Length of output: 5373
🌐 Web query:
site:curl.se/docs/manpage.html curl --retry --connect-timeout --max-time --retry-max-time semantics
💡 Result:
In curl, the following command-line options control connection behavior, execution duration, and retry logic: --connect-timeout This option specifies the maximum time in seconds allowed for the connection phase [1]. This includes DNS lookup, TCP, TLS, or QUIC handshakes [1]. If the connection is not established within this time, curl will exit [1]. --max-time This option sets the maximum time in seconds that the entire transfer operation is allowed to take. If the operation exceeds this limit, curl will terminate the transfer. --retry This option defines the number of times curl should retry a failed transfer if the failure is considered transient (e.g., connection-related errors or certain HTTP server errors). If set to 0, it will not retry. --retry-max-time This option sets an absolute time limit in seconds for the entire retry process. Regardless of how many retries remain, curl will stop retrying once this total time has elapsed since the start of the first attempt. These options can be combined to manage how curl handles unreliable networks or unresponsive servers, ensuring that individual connections fail fast while allowing for a total duration across multiple attempts [1].
Citations:
Bound the Ory download time.
Add --connect-timeout 15 --max-time 120 --retry-max-time 300. --retry 7 limits retry count but does not limit connection, transfer, or total retry time.
🤖 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 `@newsletter/action.yml` around lines 39 - 41, Add the requested curl timeout
options to the download command in the newsletter action: use a 15-second
connection timeout, 120-second maximum transfer time, and 300-second maximum
retry duration, while preserving the existing retry count and other flags.
| SSH_AUTH_SOCK: /tmp/ssh_agent.sock | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p /home/runner/.ssh | ||
| ssh-keyscan github.com >> /home/runner/.ssh/known_hosts | ||
| echo "${{ inputs.ssh_key }}" > /home/runner/.ssh/github_actions | ||
| echo "${SSH_KEY}" > /home/runner/.ssh/github_actions | ||
| chmod 600 /home/runner/.ssh/github_actions | ||
| ssh-agent -a $SSH_AUTH_SOCK > /dev/null | ||
| ssh-agent -a "$SSH_AUTH_SOCK" > /dev/null | ||
| ssh-add /home/runner/.ssh/github_actions | ||
| git config --global core.sshCommand "ssh -i /home/runner/.ssh/github_actions" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- newsletter/action.yml ---'
nl -ba newsletter/action.yml | sed -n '1,140p'
printf '%s\n' '--- SSH-related references ---'
rg -n -C 3 'SSH_AUTH_SOCK|github_actions|core\.sshCommand|ssh-agent|ssh-add|ssh-keyscan|SSH_KEY' . \
-g '!node_modules' -g '!dist' -g '!build'Repository: ory/ci
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- newsletter/action.yml ---'
awk '{printf "%6d\t%s\n", NR, $0}' newsletter/action.yml | sed -n '1,140p'
printf '%s\n' '--- SSH-related references ---'
rg -n -C 3 'SSH_AUTH_SOCK|github_actions|core\.sshCommand|ssh-agent|ssh-add|ssh-keyscan|SSH_KEY' . \
-g '!node_modules' -g '!dist' -g '!build' || trueRepository: ory/ci
Length of output: 5304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- newsletter callers ---'
rg -n -C 5 'newsletter/action\.yml|uses: .*newsletter|ssh_key:' . \
-g '*.yml' -g '*.yaml' || true
printf '%s\n' '--- newsletter implementation references ---'
rg -n -C 4 'release notify|notify send|notify draft|core\.sshCommand|git fetch|git clone' . \
-g '!node_modules' -g '!dist' -g '!build' || true
printf '%s\n' '--- cleanup and Git configuration references ---'
rg -n -C 3 'git config --global|git config --unset|ssh-agent -k|SSH_AUTH_SOCK|RUNNER_TEMP|known_hosts|github_actions' . \
-g '!node_modules' -g '!dist' -g '!build' || trueRepository: ory/ci
Length of output: 21086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("newsletter/action.yml")
text = path.read_text()
lines = text.splitlines()
checks = {
"private key is written": re.search(
r'echo "\$\{SSH_KEY\}" > (/[^ \n]+)', text
),
"fixed SSH agent socket is configured": re.search(
r'SSH_AUTH_SOCK:\s*(\S+)', text
),
"agent is started": "ssh-agent -a \"$SSH_AUTH_SOCK\"" in text,
"key is added to agent": "ssh-add /home/runner/.ssh/github_actions" in text,
"global SSH command is configured": (
'git config --global core.sshCommand "ssh -i /home/runner/.ssh/github_actions"'
in text
),
"cleanup command exists": any(
re.search(r'\b(rm|shred|unlink|git config --unset|ssh-agent -k|kill)\b', line)
for line in lines
),
"SSH_AUTH_SOCK is exported for the newsletter step": bool(
re.search(
r'- name: Draft or send the newsletter(?:(?!\n - ).)*'
r'SSH_AUTH_SOCK:',
text,
re.S,
)
),
}
for name, result in checks.items():
print(f"{name}: {'yes' if result else 'no'}")
if checks["private key is written"]:
print(f"key path: {checks['private key is written'].group(1)}")
if checks["fixed SSH agent socket is configured"]:
print(f"socket path: {checks['fixed SSH agent socket is configured'].group(1)}")
PYRepository: ory/ci
Length of output: 459
🌐 Web query:
GitHub Actions documentation composite action steps environment variables scope GITHUB_ENV
💡 Result:
In GitHub Actions, using the GITHUB_ENV environment file within a composite action allows you to define or update environment variables for subsequent steps [1]. Scope and Behavior: 1. Persistence: When you write a variable to
Citations:
- 1: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/workflow-commands.md
- 2: https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-variables
- 3: https://stackoverflow.com/questions/76522542/is-it-possible-to-inherit-the-environment-context-of-a-called-composite-github-a
- 4: https://stackoverflow.com/questions/77359560/can-we-use-env-variable-in-github-composite-actions
- 5: GITHUB_ENV does not update when running composite actions multiple times actions/runner#789
- 6: https://stackoverflow.com/questions/70370971/how-do-i-dynamically-set-an-environment-variable-in-a-composite-action-step
Clean up the SSH credential before the composite action exits.
Use unique paths under $RUNNER_TEMP, avoid git config --global, and remove the key and temporary SSH files in an if: ${{ always() }} step. The agent is not available to the newsletter step because SSH_AUTH_SOCK is step-scoped, and the configured core.sshCommand reads the key directly. Do not start the agent unless its socket and PID are passed to the required steps and the agent is stopped during cleanup.
🤖 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 `@newsletter/action.yml` around lines 49 - 58, Update the SSH setup run block
to store the key, known_hosts, and related files under unique paths in
$RUNNER_TEMP, and configure core.sshCommand locally rather than globally. Remove
ssh-agent startup and ssh-add unless the socket and PID are explicitly passed to
all required steps. Add a separate cleanup step with if: ${{ always() }} that
removes the temporary key and SSH files and stops any started agent.
| ssh-keyscan github.com >> /home/runner/.ssh/known_hosts | ||
| echo "${{ inputs.ssh_key }}" > /home/runner/.ssh/github_actions | ||
| echo "${SSH_KEY}" > /home/runner/.ssh/github_actions | ||
| chmod 600 /home/runner/.ssh/github_actions | ||
| ssh-agent -a $SSH_AUTH_SOCK > /dev/null | ||
| ssh-agent -a "$SSH_AUTH_SOCK" > /dev/null | ||
| ssh-add /home/runner/.ssh/github_actions | ||
| git config --global core.sshCommand "ssh -i /home/runner/.ssh/github_actions" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- action files ---'
git ls-files '*action.yml' '*action.yaml' | sed -n '1,120p'
printf '%s\n' '--- newsletter/action.yml context ---'
nl -ba newsletter/action.yml | sed -n '1,110p'
printf '%s\n' '--- SSH-related configuration and callers ---'
rg -n -C 3 'ssh-keyscan|known_hosts|StrictHostKeyChecking|UserKnownHostsFile|github_actions|ssh-key|newsletter' .github newsletter --glob '!**/node_modules/**' --glob '!**/dist/**' | sed -n '1,260p'Repository: ory/ci
Length of output: 4753
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- newsletter/action.yml ---'
sed -n '1,110p' newsletter/action.yml
printf '%s\n' '--- workflow references to the newsletter action ---'
rg -n -C 8 'uses:.*newsletter|newsletter/action|contents:|permissions:' .github --glob '*.yml' --glob '*.yaml' | sed -n '1,320p'
printf '%s\n' '--- checkout ordering in the action ---'
rg -n -C 12 'actions/checkout|uses:|ssh-key|working-directory|git ' newsletter/action.ymlRepository: ory/ci
Length of output: 10036
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- effective OpenSSH defaults for github.com ---'
ssh -G -o BatchMode=yes github.com 2>/dev/null |
awk '$1=="stricthostkeychecking" || $1=="userknownhostsfile" || $1=="globalknownhostsfile" || $1=="identityfile" { print }'
printf '%s\n' '--- action pin check implementation ---'
sed -n '27,75p' .github/workflows/test.yml
printf '%s\n' '--- repository-wide newsletter action references ---'
rg -n -C 5 'uses:[[:space:]]+[^#]*newsletter|newsletter/action\.yml|ssh_key:[[:space:]]*' . --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/dist/**' | sed -n '1,240p'Repository: ory/ci
Length of output: 3125
Pin GitHub’s SSH host key before configuring Git.
ssh-keyscan github.com trusts the key returned by the current network path. An attacker controlling that path can add an impostor key before later Git operations. Use GitHub’s published host key or compare the scan with a pinned fingerprint, then set StrictHostKeyChecking=yes and a job-local UserKnownHostsFile in core.sshCommand.
🤖 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 `@newsletter/action.yml` around lines 53 - 58, Update the SSH setup around
ssh-keyscan and the global core.sshCommand configuration to validate github.com
against GitHub’s published host key or a pinned fingerprint before writing
known_hosts. Configure core.sshCommand with StrictHostKeyChecking=yes and a
job-local UserKnownHostsFile, while preserving the existing key and ssh-agent
setup.
| git stash || true | ||
| git checkout master || true | ||
| git stash pop || true | ||
| git commit -a -m "autogen: update release artifacts" -m "[skip ci]" || true | ||
| git pull origin master --rebase || true | ||
| git pull "https://x-access-token:${ORY_BOT_PAT}@github.com/${GITHUB_REPOSITORY}.git" \ | ||
| master --rebase || true | ||
| if [[ "$GITHUB_REF_TYPE" == "branch" ]]; then | ||
| git push origin "${GITHUB_HEAD_REF:-$GITHUB_REF_NAME}" || true | ||
| git push "https://x-access-token:${ORY_BOT_PAT}@github.com/${GITHUB_REPOSITORY}.git" \ | ||
| "${GITHUB_HEAD_REF:-$GITHUB_REF_NAME}" || true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not hide Git failures in publication steps.
|| true lets conflicts, failed checkouts, failed rebases, and failed pushes report a successful release. The post-release path can also continue after a failed git checkout master.
releaser/action.yml#L127-L135: let checkout, stash restoration, pull, and push failures stop the action; handle only an expected no-change commit explicitly.releaser/action.yml#L163-L170: apply the same failure handling before pushingHEAD:master.
📍 Affects 1 file
releaser/action.yml#L127-L135(this comment)releaser/action.yml#L163-L170
🤖 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 `@releaser/action.yml` around lines 127 - 135, Remove blanket “|| true”
suppression from the publication Git commands in releaser/action.yml lines
127-135, allowing stash restoration, checkout master, pull/rebase, and push
failures to stop the action; handle only the expected no-change commit result
explicitly. Apply the same failure-handling correction to the push flow at
releaser/action.yml lines 163-170 before pushing HEAD:master.
| if [[ $(git describe --tags) == *"pre"* ]]; then | ||
| echo "This is a pre-release, skipping docs publishing." | ||
| exit 0 | ||
| fi | ||
|
|
||
| make post-release | ||
|
|
||
| echo "publish=true" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check the triggering release ref for pre-releases.
Lines 128 and 164 switch the worktree to master. Line 149 can then inspect a tag reachable from master, not ${GITHUB_REF_NAME}. A pre-release can publish documentation when master resolves to a non-pre-release tag.
Use the triggering ref for this check.
Proposed fix
- if [[ $(git describe --tags) == *"pre"* ]]; then
+ if [[ "$GITHUB_REF_NAME" == *"pre"* ]]; then📝 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.
| if [[ $(git describe --tags) == *"pre"* ]]; then | |
| echo "This is a pre-release, skipping docs publishing." | |
| exit 0 | |
| fi | |
| make post-release | |
| echo "publish=true" >> "$GITHUB_OUTPUT" | |
| if [[ "$GITHUB_REF_NAME" == *"pre"* ]]; then | |
| echo "This is a pre-release, skipping docs publishing." | |
| exit 0 | |
| fi | |
| make post-release | |
| echo "publish=true" >> "$GITHUB_OUTPUT" |
🤖 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 `@releaser/action.yml` around lines 149 - 155, Update the pre-release check in
the release workflow to inspect the triggering release ref `${GITHUB_REF_NAME}`
rather than relying on `git describe --tags` from the worktree switched to
master. Preserve the existing skip message and exit behavior for pre-release
refs, while allowing documentation publishing for regular releases.
1 participant
Summary
Verification
Local macOS note: the authors and changelog Make targets download a Linux ShellCheck binary, so their ShellCheck and shfmt commands were run directly with native tools.
Summary by CodeRabbit