fix: harden shell interpolation in action workflows (#74)
## Why
A user pointed out that the README pull request example interpolated
`${{ github.event.pull_request.base.ref }}` directly into a shell
command. GitHub Actions expands `${{ ... }}` expressions before the
shell runs, so copying that pattern into a workflow can let
attacker-controlled values such as branch names break shell syntax and
execute arbitrary commands.
This change hardens both the published examples and the composite action
itself so we are not shipping that pattern to users.
## What changed
- updated the README pull request review example to pass PR metadata
through `env:` and consume it as quoted shell variables
- added guidance to `docs/security.md` explaining why direct `${{ ...
}}` interpolation inside `run:` blocks is unsafe
- hardened `action.yml` so action inputs and step outputs are passed
into shell steps through environment variables instead of being spliced
directly into commands
- updated `examples/test-sandbox-protections.yml` to use the same safe
pattern
## Verification
- re-scanned this repository for attacker-controlled GitHub expressions
or action inputs interpolated directly into `run:` blocks
- parsed `action.yml` and `examples/test-sandbox-protections.yml` with
`Ruby` `YAML.load_file` to confirm the edited workflow files still parse
This commit is contained in:
@@ -37,10 +37,14 @@ jobs:
|
||||
ref: refs/pull/${{ github.event.pull_request.number }}/merge
|
||||
|
||||
- name: Pre-fetch base and head refs for the PR
|
||||
env:
|
||||
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
run: |
|
||||
# Pass GitHub expressions through env and quote shell expansions.
|
||||
git fetch --no-tags origin \
|
||||
${{ github.event.pull_request.base.ref }} \
|
||||
+refs/pull/${{ github.event.pull_request.number }}/head
|
||||
"$PR_BASE_REF" \
|
||||
"+refs/pull/$PR_NUMBER/head"
|
||||
|
||||
# If you want Codex to build and run code, install any dependencies that
|
||||
# need to be downloaded before the "Run Codex" step because Codex's
|
||||
|
||||
+64
-33
@@ -107,8 +107,10 @@ runs:
|
||||
- name: Validate Windows safety strategy
|
||||
if: ${{ runner.os == 'Windows' }}
|
||||
shell: bash
|
||||
env:
|
||||
SAFETY_STRATEGY: ${{ inputs['safety-strategy'] }}
|
||||
run: |
|
||||
if [ "${{ inputs['safety-strategy'] }}" != "unsafe" ]; then
|
||||
if [ "$SAFETY_STRATEGY" != "unsafe" ]; then
|
||||
echo "On Windows, inputs['safety-strategy'] must be 'unsafe'" >&2
|
||||
echo "because no viable sandboxing options are available at this time." >&2
|
||||
exit 1
|
||||
@@ -123,46 +125,63 @@ runs:
|
||||
|
||||
- name: Check repository write access
|
||||
env:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
GITHUB_TOKEN: ${{ github.token }}
|
||||
ALLOW_BOTS: ${{ inputs['allow-bots'] }}
|
||||
ALLOW_USERS: ${{ inputs['allow-users'] }}
|
||||
shell: bash
|
||||
run: |
|
||||
node "${{ github.action_path }}/dist/main.js" check-write-access \
|
||||
--allow-bots "${{ inputs['allow-bots'] }}" \
|
||||
--allow-users "${{ inputs['allow-users'] }}"
|
||||
node "$ACTION_PATH/dist/main.js" check-write-access \
|
||||
--allow-bots "$ALLOW_BOTS" \
|
||||
--allow-users "$ALLOW_USERS"
|
||||
|
||||
- name: Install Codex CLI
|
||||
shell: bash
|
||||
run: npm install -g "@openai/codex@${{ inputs['codex-version'] }}"
|
||||
env:
|
||||
CODEX_VERSION: ${{ inputs['codex-version'] }}
|
||||
run: npm install -g "@openai/codex@${CODEX_VERSION}"
|
||||
|
||||
- name: Install Codex Responses API proxy
|
||||
shell: bash
|
||||
run: npm install -g "@openai/codex-responses-api-proxy@${{ inputs['codex-version'] }}"
|
||||
env:
|
||||
CODEX_VERSION: ${{ inputs['codex-version'] }}
|
||||
run: npm install -g "@openai/codex-responses-api-proxy@${CODEX_VERSION}"
|
||||
|
||||
- name: Resolve Codex home
|
||||
id: resolve_home
|
||||
shell: bash
|
||||
env:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
CODEX_HOME_OVERRIDE: ${{ inputs['codex-home'] }}
|
||||
SAFETY_STRATEGY: ${{ inputs['safety-strategy'] }}
|
||||
CODEX_USER: ${{ inputs['codex-user'] }}
|
||||
CODEX_RUN_ID: ${{ github.run_id }}
|
||||
run: |
|
||||
node "${{ github.action_path }}/dist/main.js" resolve-codex-home \
|
||||
--codex-home-override "${{ inputs['codex-home'] }}" \
|
||||
--safety-strategy "${{ inputs['safety-strategy'] }}" \
|
||||
--codex-user "${{ inputs['codex-user'] }}" \
|
||||
--github-run-id "${{ github.run_id }}"
|
||||
node "$ACTION_PATH/dist/main.js" resolve-codex-home \
|
||||
--codex-home-override "$CODEX_HOME_OVERRIDE" \
|
||||
--safety-strategy "$SAFETY_STRATEGY" \
|
||||
--codex-user "$CODEX_USER" \
|
||||
--github-run-id "$CODEX_RUN_ID"
|
||||
|
||||
- name: Determine server info path
|
||||
id: derive_server_info
|
||||
shell: bash
|
||||
env:
|
||||
CODEX_HOME: ${{ steps.resolve_home.outputs.codex-home }}
|
||||
CODEX_RUN_ID: ${{ github.run_id }}
|
||||
run: |
|
||||
server_info_file="${{ steps.resolve_home.outputs.codex-home }}/${{ github.run_id }}.json"
|
||||
server_info_file="$CODEX_HOME/$CODEX_RUN_ID.json"
|
||||
echo "server_info_file=$server_info_file" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Check Responses API proxy status
|
||||
id: start_proxy
|
||||
if: ${{ inputs['openai-api-key'] != '' }}
|
||||
shell: bash
|
||||
env:
|
||||
SERVER_INFO_FILE: ${{ steps.derive_server_info.outputs.server_info_file }}
|
||||
run: |
|
||||
server_info_file="${{ steps.derive_server_info.outputs.server_info_file }}"
|
||||
if [ -s "$server_info_file" ]; then
|
||||
echo "Responses API proxy already appears to be running (found $server_info_file)."
|
||||
if [ -s "$SERVER_INFO_FILE" ]; then
|
||||
echo "Responses API proxy already appears to be running (found $SERVER_INFO_FILE)."
|
||||
echo "server_info_file_exists=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "server_info_file_exists=false" >> "$GITHUB_OUTPUT"
|
||||
@@ -175,19 +194,19 @@ runs:
|
||||
- name: Start Responses API proxy
|
||||
if: ${{ inputs['openai-api-key'] != '' && steps.start_proxy.outputs.server_info_file_exists == 'false' }}
|
||||
env:
|
||||
SERVER_INFO_FILE: ${{ steps.derive_server_info.outputs.server_info_file }}
|
||||
PROXY_API_KEY: ${{ inputs['openai-api-key'] }}
|
||||
UPSTREAM_URL: ${{ inputs['responses-api-endpoint'] }}
|
||||
shell: bash
|
||||
run: |
|
||||
upstream_url="${{ inputs['responses-api-endpoint'] }}"
|
||||
|
||||
args=(
|
||||
codex-responses-api-proxy
|
||||
--http-shutdown
|
||||
--server-info "${{ steps.derive_server_info.outputs.server_info_file }}"
|
||||
--server-info "$SERVER_INFO_FILE"
|
||||
)
|
||||
|
||||
if [ -n "$upstream_url" ]; then
|
||||
args+=(--upstream-url "$upstream_url")
|
||||
if [ -n "$UPSTREAM_URL" ]; then
|
||||
args+=(--upstream-url "$UPSTREAM_URL")
|
||||
fi
|
||||
|
||||
(
|
||||
@@ -197,22 +216,23 @@ runs:
|
||||
- name: Wait for Responses API proxy
|
||||
if: ${{ inputs['openai-api-key'] != '' && steps.start_proxy.outputs.server_info_file_exists == 'false' }}
|
||||
shell: bash
|
||||
env:
|
||||
SERVER_INFO_FILE: ${{ steps.derive_server_info.outputs.server_info_file }}
|
||||
run: |
|
||||
server_info_file="${{ steps.derive_server_info.outputs.server_info_file }}"
|
||||
for _ in {1..10}; do
|
||||
if [ -s "$server_info_file" ]; then
|
||||
if [ -s "$SERVER_INFO_FILE" ]; then
|
||||
break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
if [ ! -s "$server_info_file" ]; then
|
||||
if [ ! -s "$SERVER_INFO_FILE" ]; then
|
||||
echo "responses-api-proxy did not write server info" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ "${RUNNER_OS}" != "Windows" ]; then
|
||||
sudo chmod 444 "$server_info_file"
|
||||
sudo chown root "$server_info_file"
|
||||
sudo chmod 444 "$SERVER_INFO_FILE"
|
||||
sudo chown root "$SERVER_INFO_FILE"
|
||||
fi
|
||||
|
||||
# This step has an output named `port`.
|
||||
@@ -220,27 +240,37 @@ runs:
|
||||
id: read_server_info
|
||||
if: ${{ inputs['openai-api-key'] != '' || inputs.prompt != '' || inputs['prompt-file'] != '' }}
|
||||
shell: bash
|
||||
run: node "${{ github.action_path }}/dist/main.js" read-server-info "${{ steps.derive_server_info.outputs.server_info_file }}"
|
||||
env:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
SERVER_INFO_FILE: ${{ steps.derive_server_info.outputs.server_info_file }}
|
||||
run: node "$ACTION_PATH/dist/main.js" read-server-info "$SERVER_INFO_FILE"
|
||||
|
||||
- name: Write Codex proxy config
|
||||
if: ${{ inputs['openai-api-key'] != '' }}
|
||||
shell: bash
|
||||
env:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
CODEX_HOME: ${{ steps.resolve_home.outputs.codex-home }}
|
||||
PROXY_PORT: ${{ steps.read_server_info.outputs.port }}
|
||||
SAFETY_STRATEGY: ${{ inputs['safety-strategy'] }}
|
||||
run: |
|
||||
node "${{ github.action_path }}/dist/main.js" write-proxy-config \
|
||||
--codex-home "${{ steps.resolve_home.outputs.codex-home }}" \
|
||||
--port "${{ steps.read_server_info.outputs.port }}" \
|
||||
--safety-strategy "${{ inputs['safety-strategy'] }}"
|
||||
node "$ACTION_PATH/dist/main.js" write-proxy-config \
|
||||
--codex-home "$CODEX_HOME" \
|
||||
--port "$PROXY_PORT" \
|
||||
--safety-strategy "$SAFETY_STRATEGY"
|
||||
|
||||
- name: Drop sudo privilege, if appropriate
|
||||
if: ${{ inputs['safety-strategy'] == 'drop-sudo' && inputs['openai-api-key'] != '' }}
|
||||
shell: bash
|
||||
env:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
run: |
|
||||
case "${RUNNER_OS}" in
|
||||
Linux)
|
||||
node "${{ github.action_path }}/dist/main.js" drop-sudo --user runner --group sudo
|
||||
node "$ACTION_PATH/dist/main.js" drop-sudo --user runner --group sudo
|
||||
;;
|
||||
macOS)
|
||||
node "${{ github.action_path }}/dist/main.js" drop-sudo --user runner --group admin
|
||||
node "$ACTION_PATH/dist/main.js" drop-sudo --user runner --group admin
|
||||
;;
|
||||
*)
|
||||
echo "Unsupported OS for drop-sudo: ${RUNNER_OS}" >&2
|
||||
@@ -275,10 +305,11 @@ runs:
|
||||
CODEX_EFFORT: ${{ inputs.effort }}
|
||||
CODEX_SAFETY_STRATEGY: ${{ inputs['safety-strategy'] }}
|
||||
CODEX_USER: ${{ inputs['codex-user'] }}
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
FORCE_COLOR: 1
|
||||
shell: bash
|
||||
run: |
|
||||
node "${{ github.action_path }}/dist/main.js" run-codex-exec \
|
||||
node "$ACTION_PATH/dist/main.js" run-codex-exec \
|
||||
--prompt "${CODEX_PROMPT}" \
|
||||
--prompt-file "${CODEX_PROMPT_FILE}" \
|
||||
--output-file "$CODEX_OUTPUT_FILE" \
|
||||
|
||||
@@ -16,6 +16,20 @@ There is a lot of valuable context that can be used to fuel your invocation of C
|
||||
- **Commit messages**: a pull request can be composed of many commits. The messages for individual commits often go unnoticed, but could read by Codex.
|
||||
- **Screenshots** screenshots and other media have been known to be used as vehicles for prompt injection.
|
||||
|
||||
## Avoid shell injection in workflow steps
|
||||
|
||||
GitHub Actions expands `${{ ... }}` expressions before the shell runs your `run:` script. If you splice untrusted values such as branch names, issue titles, comment bodies, or action inputs directly into the script, those values can break shell quoting and execute arbitrary commands.
|
||||
|
||||
Instead, pass those values through `env:` and quote the shell variables that consume them:
|
||||
|
||||
```yaml
|
||||
- name: Safe shell usage
|
||||
env:
|
||||
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
|
||||
run: |
|
||||
git fetch origin "$PR_BASE_REF"
|
||||
```
|
||||
|
||||
<!-- TODO ## Protecting secrets -->
|
||||
|
||||
## Look out for API key abuse
|
||||
|
||||
@@ -29,9 +29,11 @@ jobs:
|
||||
safety-strategy: ${{ matrix.safety-strategy }}
|
||||
|
||||
- name: Try to dump the key from the codex-responses-api-proxy process
|
||||
env:
|
||||
CODEX_RUN_ID: ${{ github.run_id }}
|
||||
run: |
|
||||
# Find the PID for the codex-responses-api-proxy process.
|
||||
SERVER_INFO_FILE="$HOME/.codex/${{ github.run_id }}.json"
|
||||
SERVER_INFO_FILE="$HOME/.codex/$CODEX_RUN_ID.json"
|
||||
PID=$(jq .pid < "$SERVER_INFO_FILE")
|
||||
|
||||
# Using standard filesystem read operations (albeit privileged ones),
|
||||
|
||||
Reference in New Issue
Block a user