Skip to content

fix: template-injection: consider runner.tool_cache safe - #297

Merged
woodruffw merged 1 commit into
mainfrom
ww/runner-tool-cache-safe
Dec 14, 2024
Merged

fix: template-injection: consider runner.tool_cache safe#297
woodruffw merged 1 commit into
mainfrom
ww/runner-tool-cache-safe

Conversation

@woodruffw

Copy link
Copy Markdown
Member

Adds another always-safe context to the safe list.

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw woodruffw added the bugfix Fixes a known bug label Dec 14, 2024
@woodruffw woodruffw self-assigned this Dec 14, 2024
@woodruffw
woodruffw enabled auto-merge (squash) December 14, 2024 00:08
@woodruffw
woodruffw merged commit 8166239 into main Dec 14, 2024
@woodruffw
woodruffw deleted the ww/runner-tool-cache-safe branch December 14, 2024 00:10
@GaryJones

Copy link
Copy Markdown

Is this still working as intended? When I run zizmor with --pedantic, I get:

help[template-injection]: code injection via template expansion
  --> ./.github/workflows/integration.yml:68:39
   |
68 |         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"
   |         --- this run block            ^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
   |
   = note: audit confidence → High

Without --pendatic, there are no findings.

Either runner.tool_cache is safe, or it's not.

@woodruffw

Copy link
Copy Markdown
Member Author

Is this still working as intended? When I run zizmor with --pedantic, I get:

Yes, please see the persona docs: https://docs.zizmor.sh/usage/#using-personas

Specifically, the pedantic persona means "include code smells, even if not likely exploitable." zizmor considers all template expansions to be code smells, and puts likely non-exploitable ones under the pedantic persona so that they don't show up by default.

More information on that is documented under the audit as well:

https://docs.zizmor.sh/audits/#template-injection

(See the first tip callout in that section for context)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a known bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants