diff --git a/.github/workflows/daily-elixir-credo-snippet-audit.lock.yml b/.github/workflows/daily-elixir-credo-snippet-audit.lock.yml index dd8b212e3e1..3b3a0b65401 100644 --- a/.github/workflows/daily-elixir-credo-snippet-audit.lock.yml +++ b/.github/workflows/daily-elixir-credo-snippet-audit.lock.yml @@ -1,5 +1,5 @@ # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"6cf2dc5cf3f6169a8fc2ae8262681f558655296a60ef8bc1a96550c0f7b3520f","body_hash":"2f1463b9044ab5b109d77da64d2959d9e4394f83c383cfd35a015e481cef9acb","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.201"}} -# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"erlef/setup-beam","sha":"fc68ffb90438ef2936bbb3251622353b3dcb2f93","version":"v1.24.0"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.27","digest":"sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.27@sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27","digest":"sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27@sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27","digest":"sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27@sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.27","digest":"sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.27@sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.0","digest":"sha256:9dbdf42842c224a95016df1d2a85a2901e04204c242079343b302a307d2b8031","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.0@sha256:9dbdf42842c224a95016df1d2a85a2901e04204c242079343b302a307d2b8031"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.5.0","digest":"sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4","pinned_image":"ghcr.io/github/github-mcp-server:v1.5.0@sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4"}]} +# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"erlef/setup-beam","sha":"fc68ffb90438ef2936bbb3251622353b3dcb2f93","version":"v1.24.0"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.27","digest":"sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.27@sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27","digest":"sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27@sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27","digest":"sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27@sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.27","digest":"sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.27@sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.5.0","digest":"sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4","pinned_image":"ghcr.io/github/github-mcp-server:v1.5.0@sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # # ___ _ _ @@ -55,7 +55,7 @@ # - ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27@sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3 # - ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27@sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a # - ghcr.io/github/gh-aw-firewall/squid:0.27.27@sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409 -# - ghcr.io/github/gh-aw-mcpg:v0.4.0@sha256:9dbdf42842c224a95016df1d2a85a2901e04204c242079343b302a307d2b8031 +# - ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32 # - ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b # - ghcr.io/github/github-mcp-server:v1.5.0@sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4 @@ -542,7 +542,7 @@ jobs: GH_AW_SKILL_DIR: ".claude/skills" run: bash "${RUNNER_TEMP}/gh-aw/actions/restore_inline_skills.sh" - name: Download container images - run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.27.27@sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9 ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27@sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3 ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27@sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a ghcr.io/github/gh-aw-firewall/squid:0.27.27@sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409 ghcr.io/github/gh-aw-mcpg:v0.4.0@sha256:9dbdf42842c224a95016df1d2a85a2901e04204c242079343b302a307d2b8031 ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b ghcr.io/github/github-mcp-server:v1.5.0@sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4 + run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.27.27@sha256:bb5a0150dcff1cddf9b8045bb411b7759806bace0abcb132fb22158073e155d9 ghcr.io/github/gh-aw-firewall/api-proxy:0.27.27@sha256:01e58c4383fa9952abe76e0a134a27c970f81f744d6b7861fc9e08b7964d94c3 ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.27@sha256:70df326caf73bf5911340dca4620b529a483dd8f42142b0a41d7b9761ab4ab7a ghcr.io/github/gh-aw-firewall/squid:0.27.27@sha256:92d820df47b2eff75d93a5bec4dc183a3ec55ed7ddb4f25cb0fdda5c3e995409 ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32 ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b ghcr.io/github/github-mcp-server:v1.5.0@sha256:e25564dccc9110a70a77b9df560cbde11aa392fcb5f08b9abe5c4ebc6d146ea4 - name: Generate Safe Outputs Config run: | mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" @@ -719,7 +719,7 @@ jobs: * ) DOCKER_SOCK_PATH=/var/run/docker.sock ;; esac DOCKER_SOCK_GID=$(stat -c '%g' "$DOCKER_SOCK_PATH" 2>/dev/null || echo '0') - export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network bridge -p 127.0.0.1:'"${MCP_GATEWAY_PORT}"':'"${MCP_GATEWAY_PORT}"' --name awmg-mcpg --add-host host.docker.internal:host-gateway --user '"${MCP_GATEWAY_UID}"':'"${MCP_GATEWAY_GID}"' --group-add '"${DOCKER_SOCK_GID}"' -v '"${DOCKER_SOCK_PATH}"':/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DOCKER_HOST=unix:///var/run/docker.sock -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_POLICY_ALLOW_CREATE_PULL_REQUEST -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e RUNNER_TEMP -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw -v '"${RUNNER_TEMP}"'/gh-aw/safeoutputs:'"${RUNNER_TEMP}"'/gh-aw/safeoutputs:rw ghcr.io/github/gh-aw-mcpg:v0.4.0' + export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network bridge -p 127.0.0.1:'"${MCP_GATEWAY_PORT}"':'"${MCP_GATEWAY_PORT}"' --name awmg-mcpg --add-host host.docker.internal:host-gateway --user '"${MCP_GATEWAY_UID}"':'"${MCP_GATEWAY_GID}"' --group-add '"${DOCKER_SOCK_GID}"' -v '"${DOCKER_SOCK_PATH}"':/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DOCKER_HOST=unix:///var/run/docker.sock -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_POLICY_ALLOW_CREATE_PULL_REQUEST -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e RUNNER_TEMP -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw -v '"${RUNNER_TEMP}"'/gh-aw/safeoutputs:'"${RUNNER_TEMP}"'/gh-aw/safeoutputs:rw ghcr.io/github/gh-aw-mcpg:v0.4.1' GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) cat << GH_AW_MCP_CONFIG_7cbd56920d45855d_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" @@ -799,7 +799,7 @@ jobs: GITHUB_COPILOT_BASE_URL: ${{ env.GITHUB_COPILOT_BASE_URL }} GH_AW_NETWORK_ISOLATION: 'true' CLI_PROXY_POLICY: '{"allow-only":{"repos":"all","min-integrity":"none"}}' - CLI_PROXY_IMAGE: 'ghcr.io/github/gh-aw-mcpg:v0.4.0' + CLI_PROXY_IMAGE: 'ghcr.io/github/gh-aw-mcpg:v0.4.1' run: | bash "${RUNNER_TEMP}/gh-aw/actions/start_cli_proxy.sh" - name: Execute Claude Code CLI @@ -999,6 +999,7 @@ jobs: uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: GH_AW_AGENT_OUTPUT: /tmp/gh-aw/agent-stdio.log + GH_AW_SAFE_OUTPUTS: ${{ steps.set-runtime-paths.outputs.GH_AW_SAFE_OUTPUTS }} with: script: | const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); @@ -1020,15 +1021,7 @@ jobs: continue-on-error: true env: AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs - run: | - # Best-effort permission fix for artifact upload (AWF cleanup may not have run) - sudo -n chmod -R a+rX /tmp/gh-aw/sandbox/firewall 2>/dev/null || chmod -R a+rX /tmp/gh-aw/sandbox/firewall 2>/dev/null || true - # Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step) - if command -v awf &> /dev/null; then - awf logs summary | tee -a "$GITHUB_STEP_SUMMARY" - else - echo 'AWF binary not installed, skipping firewall log summary' - fi + run: bash "${RUNNER_TEMP}/gh-aw/actions/print_firewall_logs.sh" --rootless - name: Parse token usage for step summary if: always() continue-on-error: true diff --git a/docs/adr/44454-skip-heredoc-content-in-run-block-expression-scan.md b/docs/adr/44454-skip-heredoc-content-in-run-block-expression-scan.md new file mode 100644 index 00000000000..90c695c4ae1 --- /dev/null +++ b/docs/adr/44454-skip-heredoc-content-in-run-block-expression-scan.md @@ -0,0 +1,44 @@ +# ADR-44454: Skip Heredoc Content in Run-Block Expression Scan + +**Date**: 2026-07-09 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +`CompileSimpleWorkflow` regressed ~3x in execution time (5.67ms → 14.6ms) with ~12x more allocations after a change that introduced MCP tool support. Workflows using MCP tools generate `run:` steps that pipe JSON configuration through shell heredocs using a unique compiler-generated delimiter (e.g., `cat << GH_AW_MCP_CONFIG_01641c1bd0fd81fa_EOF | "$GH_AW_NODE" ...`). These heredoc bodies contain `${{ toJSON(...) }}` expressions that are not present in `allowedRunScriptExpressionRegex`. The text-scanning path (Path B) of `validateTemplateInjection` — `walkRunBlockLines` / `scanRunContentExpressions` — visited every line in multiline run blocks without awareness of heredoc boundaries, causing it to flag heredoc-embedded expressions as disallowed on every compilation. This set `hasDisallowed=true` unconditionally, triggering a full `yaml.Unmarshal` (~434 MB of allocations per 100-iteration benchmark run) even though the parsed path (Path A, `validateNoGitHubExpressionsInRunScriptsFromParsed`) already calls `removeHeredocContent` and never raises an error for these expressions. + +### Decision + +We will add heredoc-state tracking to `walkRunBlockLines` so that lines inside heredocs are skipped during expression scanning on Path B. This is implemented by adding `detectHeredocDelimiter` — a heuristic function that extracts the closing delimiter from a heredoc-opening line (handling unquoted, `<<-`, single-quoted, and double-quoted forms) — and threading `inHeredoc` / `heredocDelimiter` state into the walk loop. The heredoc-opening line itself is still visited (for context), but all body lines until the closing delimiter are skipped. This aligns Path B's scanning semantics with Path A's `removeHeredocContent` behavior. + +### Alternatives Considered + +#### Alternative 1: Expand `allowedRunScriptExpressionRegex` to include `toJSON(...)` patterns + +The immediate trigger was `toJSON(steps.determine-automatic-lockdown.outputs.visibility)` not matching the allowlist. Adding this specific pattern (or a broader `toJSON(...)` rule) to the regex would have eliminated the false positive for this case. However, this is a band-aid: any future expression pattern generated inside heredoc content that is not in the allowlist would re-trigger the regression. It also conflates "this expression is safe in heredoc context" with "this expression is allowed in general shell context," which are distinct security questions. Rejected because it does not fix the structural root cause. + +#### Alternative 2: Cache `yaml.Unmarshal` results per workflow content hash + +The expensive `yaml.Unmarshal` call on the `hasDisallowed` fallback path could be memoized using the raw YAML content as a cache key. This would avoid repeated parsing across multiple compilations of the same workflow. However, it introduces state (cache invalidation, memory pressure), does not fix the false-positive detection that triggers the fallback, and masks the underlying issue rather than correcting it. Rejected because caching a path that should not be taken is worse than avoiding the path entirely. + +### Consequences + +#### Positive +- `CompileSimpleWorkflow` performance is restored to baseline: ~11ms/op → ~3.8ms/op (below the historical 5.67ms/op average), and allocations drop from 71,945/op to 6,230/op +- Path B scan semantics now match Path A: both skip heredoc content, eliminating a class of false-positive expression detections for heredoc-embedded `${{ ... }}` in compiler-generated run steps +- The `detectHeredocDelimiter` function is independently testable and covered by 7 unit test cases + +#### Negative +- `detectHeredocDelimiter` is a heuristic text parser: it detects `<<` as a heredoc opener, which can produce false positives for shell comparison operators (e.g., `if [ $x < 5 ]`). The test suite covers this case and it does not trigger a false `inHeredoc` flag (since `< 5` has a space before the `<`, but `<<` does not match a single `<`), but unusual shell constructs that happen to contain `<<` in non-heredoc positions could theoretically be misidentified +- The walk loop becomes stateful (two additional variables per invocation), adding modest complexity to a previously stateless function + +#### Neutral +- The fix is localized entirely to `walkRunBlockLines` and `detectHeredocDelimiter` in `template_injection_validation.go`; no changes to the parsed-path validators or the `CompileSimpleWorkflow` API +- Tests are added for both the heredoc-skipping behavior (`TestScanRunContentExpressionsHeredoc`, 6 cases) and the delimiter extractor (`TestDetectHeredocDelimiter`, 7 cases) + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/workflow/template_injection_validation.go b/pkg/workflow/template_injection_validation.go index 9d97904ec29..f216258a5b6 100644 --- a/pkg/workflow/template_injection_validation.go +++ b/pkg/workflow/template_injection_validation.go @@ -87,12 +87,99 @@ func findRunValue(keyPart string) (string, bool) { return strings.TrimSpace(keyPart[loc[1]:]), true } +// detectHeredocDelimiter extracts the heredoc closing delimiter from a line that +// opens a heredoc (contains "<<"). It handles the common forms used in compiler- +// generated run blocks: +// +// - Unquoted: cat << DELIM ... → "DELIM" +// - Strip-tab: cat <<- DELIM ... → "DELIM" +// - Single-quoted: cat << 'DELIM' ... → "DELIM" +// - Double-quoted: cat << "DELIM" ... → "DELIM" +// +// Returns ("", false) when no heredoc opening is found on the line, including +// for bash here-strings (<<<) and arithmetic bitshifts (1 << 2). +func detectHeredocDelimiter(trimmed string) (string, bool) { + _, after, found := strings.Cut(trimmed, "<<") + if !found { + return "", false + } + // Reject bash here-strings (<<<): the character immediately after << is another <. + if len(after) > 0 && after[0] == '<' { + return "", false + } + // Handle <<- (strip-tab variant): the dash must immediately follow << with no + // intervening whitespace. Strip exactly one dash; <<-- and similar are not valid + // shell and are treated conservatively as non-heredoc. + if len(after) > 0 && after[0] == '-' { + after = after[1:] + } + rest := strings.TrimSpace(after) + if rest == "" { + return "", false + } + // Quoted delimiter: << 'DELIM' or << "DELIM" + if rest[0] == '\'' || rest[0] == '"' { + quoteChar := rest[0] + end := strings.IndexByte(rest[1:], quoteChar) + if end >= 0 { + delim := rest[1 : end+1] + if delim != "" { + return delim, true + } + } + return "", false + } + // Unquoted delimiter: take the first whitespace-delimited token. + // Require the delimiter to be a valid shell identifier (letters, digits, + // underscores; must start with a letter or underscore) to avoid treating + // arithmetic bitshifts (e.g. "1 << 2") as heredoc openings. + fields := strings.Fields(rest) + if len(fields) == 0 { + return "", false + } + delim := strings.TrimRight(fields[0], "|;&") + if !isShellIdentifier(delim) { + return "", false + } + return delim, true +} + +// isShellIdentifier reports whether s is a valid shell identifier (letters, +// digits, and underscores; must not start with a digit). Heredoc delimiters +// must match this pattern; non-matching tokens indicate non-heredoc uses of << +// such as arithmetic bitshifts. +func isShellIdentifier(s string) bool { + if s == "" { + return false + } + for i, c := range s { + switch { + case c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z', c == '_': + // always valid + case c >= '0' && c <= '9': + if i == 0 { + return false // identifiers must not start with a digit + } + default: + return false + } + } + return true +} + // walkRunBlockLines scans raw YAML text and visits each inline run value or line inside a // multiline run block. It recognizes plain run: keys as well as quoted and flow-style forms // so Path B stays aligned with the parsed-YAML validators. +// +// Heredoc content is skipped so that expressions embedded inside heredocs (which are written +// to files and never executed directly by the shell) do not trigger false positives in the +// caller's expression checks. This matches the behaviour of removeHeredocContent, which +// validateNoGitHubExpressionsInRunScriptsFromParsed uses on the parsed path. func walkRunBlockLines(yamlContent string, visit func(line string) bool) bool { inRunBlock := false runBlockIndent := 0 + inHeredoc := false + heredocDelimiter := "" for line := range strings.SplitSeq(yamlContent, "\n") { trimmed := strings.TrimLeft(line, " \t") @@ -104,8 +191,29 @@ func walkRunBlockLines(yamlContent string, visit func(line string) bool) bool { if inRunBlock { if indent <= runBlockIndent { inRunBlock = false + inHeredoc = false + heredocDelimiter = "" // Fall through: check whether this line starts a new run: block. } else { + // If we are inside a heredoc, look for the closing delimiter. + if inHeredoc { + if strings.TrimSpace(line) == heredocDelimiter { + inHeredoc = false + heredocDelimiter = "" + } + continue // always skip heredoc content + } + // Check whether this line opens a heredoc. + if delim, ok := detectHeredocDelimiter(trimmed); ok { + inHeredoc = true + heredocDelimiter = delim + // The opening line itself is not heredoc body; visit it so callers + // can see the surrounding shell context, but do not recurse. + if visit(line) { + return true + } + continue + } if visit(line) { return true } diff --git a/pkg/workflow/template_injection_validation_test.go b/pkg/workflow/template_injection_validation_test.go index 362df3d42ca..4c2ebd646dd 100644 --- a/pkg/workflow/template_injection_validation_test.go +++ b/pkg/workflow/template_injection_validation_test.go @@ -1405,3 +1405,191 @@ func TestScanRunContentExpressions(t *testing.T) { }) } } + +// TestScanRunContentExpressionsHeredoc verifies that expressions inside heredocs +// are not flagged as disallowed – this is the core fix for the CompileSimpleWorkflow +// performance regression where ${{ toJSON(steps.determine-automatic-lockdown...) }} +// inside a heredoc was triggering a full yaml.Unmarshal on every compilation. +func TestScanRunContentExpressionsHeredoc(t *testing.T) { + tests := []struct { + name string + yaml string + wantHasUnsafe bool + wantHasDisallowed bool + }{ + { + name: "disallowed expression inside unquoted heredoc is not flagged", + yaml: `jobs: + test: + steps: + - run: | + cat << GH_AW_MCP_CONFIG_EOF | node start_mcp.cjs + { + "sink-visibility": ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }} + } + GH_AW_MCP_CONFIG_EOF`, + wantHasUnsafe: false, + wantHasDisallowed: false, + }, + { + name: "unsafe expression inside unquoted heredoc is not flagged", + yaml: `jobs: + test: + steps: + - run: | + cat << EOF + title: ${{ github.event.issue.title }} + EOF`, + wantHasUnsafe: false, + wantHasDisallowed: false, + }, + { + name: "disallowed expression OUTSIDE heredoc is still flagged", + yaml: `jobs: + test: + steps: + - run: | + echo ${{ github.actor }} + cat << EOF + safe: content + EOF`, + wantHasUnsafe: false, + wantHasDisallowed: true, + }, + { + name: "unsafe expression OUTSIDE heredoc is still flagged", + yaml: `jobs: + test: + steps: + - run: | + echo "${{ github.event.issue.title }}" + cat << EOF + safe + EOF`, + wantHasUnsafe: true, + wantHasDisallowed: true, + }, + { + name: "allowed expression after heredoc close is not flagged", + yaml: `jobs: + test: + steps: + - run: | + cat << EOF + safe + EOF + node ${{ runner.temp }}/actions/foo.cjs`, + wantHasUnsafe: false, + wantHasDisallowed: false, + }, + { + name: "quoted heredoc delimiter (single-quoted) skips content", + yaml: `jobs: + test: + steps: + - run: | + cat << 'EOF' + literal: ${{ github.event.issue.title }} + EOF`, + wantHasUnsafe: false, + wantHasDisallowed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := scanRunContentExpressions(tt.yaml) + + assert.Equal(t, tt.wantHasUnsafe, got.hasUnsafe, "hasUnsafe mismatch") + assert.Equal(t, tt.wantHasDisallowed, got.hasDisallowed, "hasDisallowed mismatch") + }) + } +} + +// TestDetectHeredocDelimiter verifies the heredoc delimiter extraction function. +func TestDetectHeredocDelimiter(t *testing.T) { + tests := []struct { + name string + line string + wantDelim string + wantOK bool + }{ + { + name: "unquoted delimiter", + line: `cat << EOF | node script.cjs`, + wantDelim: "EOF", + wantOK: true, + }, + { + name: "compiler-style delimiter with prefix", + line: `cat << GH_AW_MCP_CONFIG_01641c1bd0fd81fa_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/actions/start.cjs"`, + wantDelim: "GH_AW_MCP_CONFIG_01641c1bd0fd81fa_EOF", + wantOK: true, + }, + { + name: "single-quoted delimiter", + line: `cat << 'EOF'`, + wantDelim: "EOF", + wantOK: true, + }, + { + name: "double-quoted delimiter", + line: `cat << "MY_DELIM"`, + wantDelim: "MY_DELIM", + wantOK: true, + }, + { + name: "strip-tab variant", + line: `cat <<- EOF`, + wantDelim: "EOF", + wantOK: true, + }, + { + name: "no heredoc", + line: `echo hello`, + wantDelim: "", + wantOK: false, + }, + { + name: "less-than operator is not a heredoc", + line: `if [ $x < 5 ]; then`, + wantDelim: "", + wantOK: false, + }, + { + name: "bash here-string is not a heredoc", + line: `cat <<< "hello world"`, + wantDelim: "", + wantOK: false, + }, + { + name: "arithmetic bitshift is not a heredoc", + line: `echo $((1 << 2))`, + wantDelim: "", + wantOK: false, + }, + { + name: "arithmetic bitshift no spaces is not a heredoc", + line: `result=$((flags<<4))`, + wantDelim: "", + wantOK: false, + }, + { + name: "delimiter starting with dash is not a heredoc", + line: `cat << -EOF`, + wantDelim: "", + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + trimmed := strings.TrimLeft(tt.line, " \t") + gotDelim, gotOK := detectHeredocDelimiter(trimmed) + assert.Equal(t, tt.wantOK, gotOK) + if tt.wantOK { + assert.Equal(t, tt.wantDelim, gotDelim) + } + }) + } +}