Skip to content

Commit 12c982f

Browse files
fix: preserve env var case in template injection fixes (#1766)
* fix: preserve env var case in template injection fixes `context_to_env_var()` unconditionally uppercased all generated env var names. For `env.*` contexts this changes the variable name (e.g. `env.build_tag` → `BUILD_TAG`), silently breaking workflows on Linux where env vars are case-sensitive. Only uppercase for non-env contexts, where we're creating new variable names. For env contexts, preserve the original casing. Fixes #1765 * Record changes Signed-off-by: William Woodruff <[email protected]> --------- Signed-off-by: William Woodruff <[email protected]> Co-authored-by: William Woodruff <[email protected]>
1 parent cfafaf3 commit 12c982f

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

crates/zizmor/src/audit/template_injection.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,8 @@ impl TemplateInjection {
216216
// TODO: We could probably go a step further here and avoid the
217217
// loop below entirely, since we expect `env` contexts to only
218218
// have a single part, i.e. `foo.bar` and never `foo.bar.baz`.
219-
if ctx.child_of("env") {
219+
let is_env_context = ctx.child_of("env");
220+
if is_env_context {
220221
ctx_parts.next();
221222
}
222223

@@ -271,7 +272,16 @@ impl TemplateInjection {
271272
}
272273
}
273274

274-
Some(env_parts.join("_").to_uppercase())
275+
let env_var = env_parts.join("_");
276+
277+
// For `env` contexts, preserve the original case since these refer
278+
// to existing environment variables. GitHub recommends treating
279+
// environment variables as case sensitive.
280+
if is_env_context {
281+
Some(env_var)
282+
} else {
283+
Some(env_var.to_uppercase())
284+
}
275285
}
276286

277287
/// Attempts to produce a `Fix` for a given expression.
@@ -1059,7 +1069,7 @@ jobs:
10591069
echo "User: ${GITHUB_ACTOR}"
10601070
echo "User: ${GITHUB_ACTOR}"
10611071
echo "User: ${GITHUB_ACTOR}"
1062-
echo "User: ${GITHUB_ACTOR}"
1072+
echo "User: ${github_actor}"
10631073
"#);
10641074
}
10651075
);
@@ -1173,11 +1183,11 @@ jobs:
11731183
// Computed indices not supported
11741184
("foo.bar[computed]", None),
11751185
("foo.bar[abc && def]", None),
1176-
// env contexts should have the env prefix removed
1186+
// env contexts should have the env prefix removed and preserve case
11771187
("env.FOO_BAR", Some("FOO_BAR")),
1178-
("env.foo_bar", Some("FOO_BAR")),
1179-
("ENV.foo_bar", Some("FOO_BAR")),
1180-
("ENV['foo_bar']", Some("FOO_BAR")),
1188+
("env.foo_bar", Some("foo_bar")),
1189+
("ENV.foo_bar", Some("foo_bar")),
1190+
("ENV['foo_bar']", Some("foo_bar")),
11811191
("env.GITHUB_ACTOR", Some("GITHUB_ACTOR")),
11821192
// FIXME: soundness hole
11831193
(

docs/release-notes.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ of `zizmor`.
2828
* The [dependabot-cooldown] audit no longer flags missing cooldowns for
2929
the `pre-commit` ecosystem, which does not support cooldown configuration (#1758)
3030

31+
* Fixed a bug where auto-fixes for the [template-injection] audit would fail
32+
to preserve an environment variable's casing (#1766)
33+
3134
## 1.23.1
3235

3336
### Bug Fixes 🐛

0 commit comments

Comments
 (0)