From 39b483218fec14516b4a7e07707aa63f9acc87ad Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 16 Mar 2026 16:13:54 -0700 Subject: [PATCH] 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 --- README.md | 8 ++- action.yml | 97 ++++++++++++++++++--------- docs/security.md | 14 ++++ examples/test-sandbox-protections.yml | 4 +- 4 files changed, 87 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index bccb3d9..7560878 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/action.yml b/action.yml index d83acc2..c1cb84f 100644 --- a/action.yml +++ b/action.yml @@ -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" \ diff --git a/docs/security.md b/docs/security.md index 1c86388..8bdec4b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -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" +``` + ## Look out for API key abuse diff --git a/examples/test-sandbox-protections.yml b/examples/test-sandbox-protections.yml index 8347f26..5c2b80e 100644 --- a/examples/test-sandbox-protections.yml +++ b/examples/test-sandbox-protections.yml @@ -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),