Fix GitHub Actions: Prevent Command Injection in Firefox Workflow

[PeterDaveHello]

Motivation

• Detected that .github/workflows/firefox-metadata.yml directly expands workflow_dispatch.inputs.version into a shell run step, which can lead to command injection and potentially expose FIREFOX_* secrets.
• The goal is to eliminate the injection risk with minimal changes while preserving the existing Firefox metadata update workflow.

Description

• Changed the workflow to pass the user input through the FIREFOX_METADATA_VERSION environment variable and reference it in the shell as "$FIREFOX_METADATA_VERSION" to prevent GitHub Actions expressions from being expanded before shell parsing.
• Added AMO-compatible numeric version validation for x.y.z and x.y.z.w, allowing up to nine digits per component while rejecting leading zeros. The step aborts before running npm run release:update-firefox-metadata if the input does not match.
• Explicitly selected Bash for the step that uses [[ ... =~ ... ]] validation.
• Modified file: .github/workflows/firefox-metadata.yml.

Testing

• Ran npm test; all automated unit tests passed with no failures.
• Ran npm run lint; static analysis passed with no lint errors.
• Ran npm run build and verified that the expected artifacts exist in build/chromium/, and that git diff --check reported no issues.

---

Codex Task

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes
  • Improved Firefox metadata updates by validating the provided Firefox version format before running the update (supports both x.y.z and x.y.z.w).
  • The workflow now fails fast when the version is malformed, reducing the risk of incorrect release metadata.
  • Added an example to the manual version input to clarify the accepted four-part version format.

[gemini-code-assist]

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b33b378a-91d0-4c19-85f0-d6bb0cd0547d

📥 Commits

Reviewing files that changed from the base of the PR and between dd8d650 and 254ad52.

📒 Files selected for processing (1)

• .github/workflows/firefox-metadata.yml

🚧 Files skipped from review as they are similar to previous changes (1)

• .github/workflows/firefox-metadata.yml

---

📝 Walkthrough

Walkthrough

The Firefox metadata workflow documents three- or four-component version formats and validates the supplied version before running the update command. Invalid values stop the step, and valid values pass to npm through FIREFOX_METADATA_VERSION.

Changes

Workflow Version Validation

Layer / File(s)	Summary
Document and validate Firefox version input .github/workflows/firefox-metadata.yml	The workflow adds a four-component version example. It validates three- or four-component numeric versions without leading zeros before invoking the metadata update command.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: josstorer

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing command injection in the Firefox GitHub Actions workflow.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/fix-command-injection-vulnerability-in-workflow

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Harden Firefox metadata GitHub Action against command injection

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

AI Description

• Pass workflow input via env var and quote it in bash to prevent shell injection.
• Add strict x.y.z semver validation and fail fast on invalid versions.
• Keep existing npm-based Firefox metadata update flow unchanged otherwise.

Diagram

graph TD
  A["workflow_dispatch input: version"] --> B["Env: FIREFOX_METADATA_VERSION"] --> C{"Semver x.y.z?"} -->|"yes"| D["Run npm update script"] --> E["Update Firefox metadata"]
  C -->|"no"| F["Exit 1 + error"]
  G[("FIREFOX_* secrets")] --> D

  subgraph Legend
    direction LR
    _in["Input"] ~~~ _dec{"Decision"} ~~~ _proc["Process"] ~~~ _sec[("Secrets")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Validate with a semver tool (node/semver or npx semver)

• ➕ More accurate semver handling (pre-releases/build metadata if desired)
• ➕ Clearer error messages and maintainability vs bash regex
• ➖ Adds dependency/tooling surface area to a workflow step
• ➖ Slightly more moving parts for a minimal security fix

2. Use workflow_dispatch input constraints (choices) when feasible

• ➕ Prevents invalid values before the runner starts
• ➕ No shell parsing involved for validation
• ➖ Not practical for arbitrary version values (needs enumerated list)
• ➖ Still should quote/handle safely in the step

3. Add hardened shell defaults (set -euo pipefail) and explicit shell

• ➕ Improves safety and debuggability across the step
• ➕ Reduces risk from unset variables or silent failures
• ➖ Can be a behavioral change if the script currently relies on unset vars
• ➖ Still requires proper quoting/validation of inputs

Recommendation: The PR’s approach (env indirection + strict quoting + allowlist validation) is the best minimal-change mitigation for command injection in a workflow run step. Consider optionally adding set -euo pipefail for additional robustness, but the core security issue is already addressed effectively.

Files changed (1) +8 / -1

Bug fix (1) +8 / -1

firefox-metadata.ymlAvoid shell injection from dispatch input and validate version +8/-1

Avoid shell injection from dispatch input and validate version

• Replaces direct interpolation of 'inputs.version' in the 'run' command with an environment variable ('FIREFOX_METADATA_VERSION') used in a quoted context. Adds a strict x.y.z semver regex check to fail fast on invalid input before running the npm metadata update script.

.github/workflows/firefox-metadata.yml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 6 rules

1. Implicit bash dependency ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

此步驟新增了 bash 專用的 [[ ... =~ ... ]] 正則判斷，但 workflow/step 未明確指定 shell: bash，未來若 workflow 預設 shell
被調整（或重用到非 bash 環境）會導致語法錯誤而使更新步驟失敗。

Code

.github/workflows/firefox-metadata.yml[R30-34]

+        run: |
+          if [[ ! "$FIREFOX_METADATA_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
+            echo "Firefox version must use x.y.z semver format" >&2
+            exit 1
+          fi

Evidence

在本 PR 新增的 run 腳本中使用了 [[ ... =~ ... ]]；同一個 workflow 檔案中並未看到 step-level shell: 或
defaults.run.shell 設定，因此這段腳本對預設 shell 行為形成隱含依賴。

.github/workflows/firefox-metadata.yml[14-41]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow step relies on bash-specific syntax (`[[ ... =~ ... ]]`) but does not explicitly pin the shell. This creates a portability/maintenance hazard: if the workflow’s default shell changes or the job is later adapted to a different runner/shell, the step may fail with a syntax error.

### Issue Context
The current step uses `[[` and `=~` for regex validation. These are not POSIX `sh` features.

### Fix Focus Areas
- .github/workflows/firefox-metadata.yml[30-36]

### Suggested fix
Add `shell: bash` to the step (or define `defaults: run: shell: bash` for the job/workflow), **or** rewrite the validation in POSIX-compatible `sh` (e.g., using `case`/`grep -E`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[Copilot]

Pull request overview

This pull request hardens the Firefox metadata GitHub Actions workflow against command injection by ensuring the user-provided workflow_dispatch version input is treated as data (not shell code) and is validated before use.

Changes:

• Routes the inputs.version value through an environment variable and references it as "$FIREFOX_METADATA_VERSION" in the shell step.
• Adds strict x.y.z semantic version validation (^[0-9]+\.[0-9]+\.[0-9]+$) and fails fast on invalid input.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)

• .github/workflows/firefox-metadata.yml

---

Incremental review note: the prior inline concern about the implicit bash dependency (use of [[ ... =~ ... ]]) is now resolved by the addition of shell: bash on the step. The version input is now passed via the FIREFOX_METADATA_VERSION environment variable and validated against a strict numeric format before use, eliminating the command-injection risk.

Previous Review Summaries (2 snapshots, latest commit 2f820a0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 2f820a0)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)

• .github/workflows/firefox-metadata.yml

Previous review (commit 79436f1)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)

• .github/workflows/firefox-metadata.yml

---

_{Reviewed by hy3:free · Input: 22.5K · Output: 1.9K · Cached: 136.2K}

[PeterDaveHello]

@josStorer would you like to take a look at this PR?

[gemini-code-assist]

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

[qodo-code-review]

Code review by qodo was updated up to the latest commit dd8d650

[coderabbitai]

🧹 Nitpick comments (1)

.github/workflows/firefox-metadata.yml (1)

18-19: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Disable credential persistence to improve security posture.

If npm run release:update-firefox-metadata does not push commits back to the repository, you can safely disable credential persistence in the checkout action. This adheres to the principle of least privilege by preventing subsequent steps or dependencies from unnecessarily accessing the GitHub token in the runner environment.

🛡️ Proposed fix to disable credential persistence

       - name: Checkout
         uses: actions/checkout@v7
+        with:
+          persist-credentials: false

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/firefox-metadata.yml around lines 18 - 19, Update the
Checkout step using actions/checkout@v7 to disable credential persistence by
setting persist-credentials to false, preserving the existing checkout behavior.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/firefox-metadata.yml:
- Around line 18-19: Update the Checkout step using actions/checkout@v7 to
disable credential persistence by setting persist-credentials to false,
preserving the existing checkout behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7eb36851-454e-41d3-82f3-a745e6c0d4c0

📥 Commits

Reviewing files that changed from the base of the PR and between 2f820a0 and dd8d650.

📒 Files selected for processing (1)

• .github/workflows/firefox-metadata.yml

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 6 rules

1. Long bash lines over 100 📘 Rule violation ⚙ Maintainability

Description

The added Bash validation step includes lines that exceed 100 characters, violating the repository
line-length requirement. This can reduce readability and may conflict with formatting/linting
expectations for changed files.

Code

.github/workflows/firefox-metadata.yml[R32-33]

+          if [[ ! "$FIREFOX_METADATA_VERSION" =~ ^(0|[1-9][0-9]{0,8})(\.(0|[1-9][0-9]{0,8})){2,3}$ ]]; then
+            echo "Firefox version must use x.y.z or x.y.z.w numeric format without leading zeros" >&2

Evidence

PR Compliance ID 2261946 requires that no non-comment, non-whitespace line exceed 100 characters.
The workflow adds a long [[ ... =~ ... ]] regex line and a long echo message line, both
exceeding 100 characters in the updated file.

Rule 2261946: Limit source line length to 100 characters
.github/workflows/firefox-metadata.yml[32-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New/modified lines in the workflow exceed the 100-character maximum line length.

## Issue Context
The `if [[ ... =~ ... ]]` regex and the `echo` error message are each on a single long physical line. The compliance rule requires each physical (non-comment, non-whitespace) line to be 100 characters or fewer.

## Fix Focus Areas
- .github/workflows/firefox-metadata.yml[32-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-code-review]

1. Long bash lines over 100 📘 Rule violation ⚙ Maintainability

The added Bash validation step includes lines that exceed 100 characters, violating the repository
line-length requirement. This can reduce readability and may conflict with formatting/linting
expectations for changed files.

Agent Prompt

## Issue description
New/modified lines in the workflow exceed the 100-character maximum line length.

## Issue Context
The `if [[ ... =~ ... ]]` regex and the `echo` error message are each on a single long physical line. The compliance rule requires each physical (non-comment, non-whitespace) line to be 100 characters or fewer.

## Fix Focus Areas
- .github/workflows/firefox-metadata.yml[32-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools