Skip to content

[Bug] Validate tag keys/values for deploy/destroy tools (shell + JMESPath safety) #3

Description

@sjohnston1972

Part of the command-injection hardening parent issue.

Background

Beyond scalar inputs, the deploy/destroy tools interpolate tag keys and values into shell scripts and into JMESPath query strings. See custom-tools.ts:

  • runDestroy (~604): tag filters interpolated into an echo (~633) and into a JMESPath filter tags.\"${k}\"=='${v}' (~642-645), reused in the standalone-resource query and the wait loop.
  • runBicepDeploy (~750): required_tags interpolated into az tag update ... --tags '${k}=${v}' (~839-857).
  • runDestroyAws (~1250): tag_filters into Tags[?Key=='${k}' && Value=='${v}'] (~1287) and an echo (~1276).

Problem / Goal

Validate every tag key and value so they cannot break out of either the shell single/double quotes or the JMESPath string literal. Azure/AWS tag keys and values have limited legal character sets anyway, so a strict allowlist is both safe and correct.

Where to look

Same three handlers above. Note the same tag map is used in two contexts (shell and JMESPath), so a single validator must reject metacharacters dangerous to both: ', ", backtick, $, ;, \, and newlines.

Suggested approach

  1. Add a validateTags(tags: Record<string,string>): string | null helper returning an error message or null.
    • Reject empty maps where the caller requires at least one entry (coordinate with the destroy-safety parent issue — do not silently allow an empty effective filter).
    • Allow a conservative set, e.g. keys /^[a-zA-Z0-9._-]{1,128}$/, values /^[a-zA-Z0-9 ._:\/-]{0,256}$/. Explicitly reject single quote, double quote, backtick, $, ;, \, and control chars.
  2. Call it at the top of runDestroy, runBicepDeploy, and runDestroyAws; on failure return is_error: true with the offending key/value named, before any docker spawn.

Acceptance criteria

  • Tag keys/values containing quote, backtick, $, ;, \, or newline are rejected with a clear error and spawn no container.
  • Legitimate tags (mcp-project=lab, mcp-topology-id=<uuid>, mcp-deployed-at=2026-07-07T00:00:00Z) pass unchanged.

Testing

  • Unit-test validateTags: legit tag sets pass; each dangerous character class is rejected in both key and value position.

Out of scope

  • Scalar validation (separate sub-issue).
  • The env-var refactor (separate sub-issue) — validation here is defence-in-depth on top of it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions