Skip to content

Allow colons in model IDs (versioned IDs and inference profiles)#4

Merged
Andrey9kin merged 2 commits into
mainfrom
2026-07-04-model-id-colon-validation
Jul 4, 2026
Merged

Allow colons in model IDs (versioned IDs and inference profiles)#4
Andrey9kin merged 2 commits into
mainfrom
2026-07-04-model-id-colon-validation

Conversation

@Andrey9kin

Copy link
Copy Markdown
Member

Summary

  • Relax the model / subagent_model validation regexes from [A-Za-z0-9._-]+ to [A-Za-z0-9._:-]+ in both spots (repo-config sanitizer and subagent-model check) — Bedrock model IDs can carry a version suffix like anthropic.claude-3-5-sonnet-20241022-v2:0, which the old regex rejected
  • Update the model input description to say "Bedrock model or inference profile ID"

Upstreamed from the vendored copy in sirob-tech/.github (diverged when it was pointed at Opus). The zai.glm-5 default is intentionally unchanged; the org-specific caller wiring there was not brought back.

🤖 Generated with Claude Code

Bedrock model IDs can carry a version suffix (e.g.
anthropic.claude-3-5-sonnet-20241022-v2:0), which the model/subagent_model
validation regexes rejected. Relax both to accept ':' and note in the
input description that inference profile IDs work too.

Upstreamed from the vendored copy in sirob-tech/.github. The zai.glm-5
default stays unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# that reaches prompts or CLI flags to prevent injection.
raw_model = str(cfg.get("model") or model)
if re.fullmatch(r"[A-Za-z0-9._-]+", raw_model):
if re.fullmatch(r"[A-Za-z0-9._:-]+", raw_model):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex does not allow forward slashes, which are required for application inference profile ARNs (e.g., arn:aws:bedrock:us-east-1:123456789012:application-inference-profile/my-profile). This could prevent callers from using application inference profiles even though the description mentions 'inference profile ID'. Consider if / should be added to the allowed character set, or clarify in the description which inference profile formats are supported.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 52b942c by clarifying the description instead of widening the charset: the input now states that cross-region inference profile IDs (e.g. us.anthropic.claude-opus-4-8) are supported and ARN-style application inference profiles are not. Keeping / out of the sanitizer preserves the tight injection surface, and OpenCode's amazon-bedrock provider hasn't been verified to accept ARNs as model refs.

@fivexl-reviewer

fivexl-reviewer Bot commented Jul 4, 2026

Copy link
Copy Markdown

AI Code Review Summary

  • Findings posted: 0 inline, 0 thread replies
  • Threads marked resolved: 0

Additional observations

  • The regex change from [A-Za-z0-9.-]+ to [A-Za-z0-9.:-]+ is secure and correctly adds colon support for cross-region inference profile IDs like us.anthropic.claude-opus-4-8. The existing comment about forward slashes for ARN-style profiles remains relevant - ARNs are still not supported by design. Both Python (line 271) and Bash (line 858) validations are consistent. No blocking issues found.

Note for AI coding agents: detailed findings are posted as inline review comments on the diff — read the unresolved review threads (e.g. gh api repos/<owner>/<repo>/pulls/<pr>/comments) before making changes; this summary is only an overview.

This summary was automatically generated by the AI code review workflow.

Scope the description to cross-region inference profile IDs; ARN-style
application inference profiles stay outside the sanitizer's charset.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Andrey9kin
Andrey9kin merged commit 056088b into main Jul 4, 2026
1 check passed
@Andrey9kin
Andrey9kin deleted the 2026-07-04-model-id-colon-validation branch July 4, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant