Skip to content

--model value not quoted in claude_args assembly — benign now but fragile if charset ever widens #144

Closed

Description

@twistedmelonman

Non-Blocking Review Concern: --model value not quoted in claude_args assembly — benign now but fragile if charset ever widens

Source: pre-push whole-codebase review
Location: .github/workflows/claude-assistant.yml:178
Date: 2026-08-17

What was flagged

CLAUDE_ARGS="--model $MODEL" emits the model name without quoting. Currently safe because the validation regex [a-zA-Z0-9._-]+ excludes all whitespace and shell-special characters, so there's no injection path. But the assembled string is later passed as a step output and expanded unquoted in the YAML claude_args: ${{ steps.args.outputs.claude_args }} value. If the charset validation were ever relaxed (e.g., to allow / for a hypothetical model path), the unquoted $MODEL would word-split. Low risk as written, but defensive quoting (CLAUDE_ARGS="--model \"$MODEL\"") would make this immune to future charset drift.

Context

This issue was automatically created from a non-blocking concern identified
during pre-push whole-codebase review. It was flagged for tracking.


Created by lib-review-issues.sh

Activity

  1. twistedmelonman commented on Aug 18, 2026

    @twistedmelonman
    MemberAuthor

    Fixed in #149 (merged 4127f03). Verified on main:

    220:            CLAUDE_ARGS="--model \"$MODEL\""
    

    Now consistent with how allowed_tools and disallowed_tools are assembled.

    For the record, this was defensive rather than a live bug: model is charset-validated before use, so the unquoted form could not actually break. The quoting guards against future charset drift.

    Related: #148 claimed this same change embeds literal backslashes that reach the action untested. It does not — the \" is bash source-level escaping consumed at parse time. Verified empirically:

    result: --model "claude-sonnet-4-6" --allowed-tools "..." --disallowed-tools "..."
    backslash count: 0

    #148 was closed on that basis.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtTechnical debt to address

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions