Uh oh!
There was an error while loading. Please reload this page.
feat: add --config and --steps CLI flags for programmatic setup - #2545
Conversation
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: CHANGES REQUESTED
Commit: 9d7932c
Findings
MEDIUM - packages/cli/src/shared/spawn-config.ts:35 — Null byte check occurs after path resolution
The null byte check happens after resolve(filePath) is called. While Node.js will reject null bytes in paths, the check should come before any filesystem operations for defense-in-depth. Move line 35-37 before line 32.
LOW - Multiple files — Missing input validation on model ID and steps
The --model and --steps flags accept arbitrary strings without validation. While this is non-critical (values are validated later or unused), consider adding basic format validation:
- Model ID should match pattern:
provider/model-name - Steps should be validated before being set in env vars (already done in orchestrate.ts, but could be earlier)
INFO - packages/cli/src/index.ts:846-850 — Credentials in environment variables
The PR correctly loads TELEGRAM_BOT_TOKEN and GITHUB_TOKEN from config files into env vars. This is acceptable for spawn's design (credentials are already in env vars), but the config file path should be validated to prevent directory traversal.
INFO - packages/cli/src/shared/agent-setup.ts:800-815 — Token escaping is correct
The code uses jsonEscape() (which calls JSON.stringify()) for the Telegram bot token before passing to shell. This is correct and prevents command injection.
Tests
- bash -n: PASS (sh/e2e/lib/verify.sh syntax valid)
- bun test: PASS (1405 tests pass, 0 fail)
- curl|bash: OK (no violations found in shell scripts)
- macOS compat: OK (no bash 3.x incompatibilities)
Additional Issues
- Merge conflicts: The PR shows
mergeable: CONFLICTINGstatus. Please rebase on main and resolve conflicts. - Documentation file: The PR adds
.claude/rules/agent-setup-options.mdwhich violates the Documentation Policy in CLAUDE.md. Per the policy, only README.md, CLAUDE.md, and cloud-specific sh/{cloud}/README.md are allowed. Move this to.docs/(git-ignored).
Recommendations
Required before merge:
- Rebase on main and resolve merge conflicts
- Move
.claude/rules/agent-setup-options.mdto.docs/agent-setup-options.md - Move null byte check before path resolution (line 35 → before line 32)
Optional improvements:
4. Add model ID format validation in index.ts
5. Add early steps validation in index.ts (before orchestrate.ts)
-- security/pr-reviewer
9d7932c to
4bb28f5CompareAdds separate "Telegram" and "WhatsApp" checkboxes to the OpenClaw setup screen: - Telegram: prompts for bot token from @Botfather, injects into OpenClaw config via `openclaw config set` - WhatsApp: reminds user to scan QR code via the web dashboard after launch (no CLI setup possible) Updates USER.md with channel-specific guidance when either is selected. Bump CLI version to 0.16.16. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of punting WhatsApp setup to "after launch", runs `openclaw channels login --channel whatsapp` as an interactive SSH session between gateway start and TUI launch. The user scans the QR code with their phone during provisioning setup. Flow: gateway starts → tunnel set up → WhatsApp QR scan → TUI launch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add --config <path> flag to load spawn options from a JSON config file (model, steps, name, setup data like telegram_bot_token). Add --steps <list> flag for comma-separated setup step control. Both enable the web UI and headless automation to control which setup steps run. Priority order: CLI flags > --config file > env vars > defaults. - New spawn-config.ts module with valibot validation - OptionalStep extended with dataEnvVar and interactive metadata - validateStepNames() for step name validation with warnings - Telegram setup reads TELEGRAM_BOT_TOKEN env var before prompting - WhatsApp auto-skipped in headless mode with warning - promptSetupOptions() skipped when SPAWN_ENABLED_STEPS already set - E2E verify helpers for github, browser, telegram setup artifacts - QA reference file documenting all agent setup options - Version bump to 0.17.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add --model <id> CLI flag that sets MODEL_ID env var - --model is extracted before --config so it takes priority - Add config-priority.test.ts with 8 tests verifying: - --model overrides config model - --steps overrides config steps - --steps "" disables all steps - --name overrides config name - Config tokens apply as defaults - Explicit env vars override config tokens - Remove preferences.json from priority order docs (not needed) - Add --model to help text and unknown-flag guidance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document config file format, setup steps table, and new CLI flags in the commands table. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move null byte check before path resolution (defense-in-depth) - Move agent-setup-options.md from .claude/rules/ to .docs/ (git-ignored) per documentation policy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rebase on main introduced a duplicate --model flag extraction block (one from the PR at line 804, one from main at line 941). Consolidated into the single early extraction point with -m shorthand support. Also removed duplicate --model entry from KNOWN_FLAGS set. Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
4bb28f5 to
5a48de2Comparela14-1
commented
Mar 12, 2026
Rebase + conflict resolutionRebased onto
Review feedback statusThe existing commit
Verification
-- refactor/pr-maintainer |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 5a48de2
Summary
All security concerns from the previous review (commit 9d7932c) have been successfully addressed:
- ✓ Null byte check - Moved to line 33 in spawn-config.ts, BEFORE path resolution (defense-in-depth)
- ✓ Documentation policy - agent-setup-options.md removed from .claude/rules/ and moved to .docs/ (git-ignored)
- ✓ Merge conflicts - Resolved (mergeable: MERGEABLE)
Security Findings
No issues found. The PR implements secure credential handling:
- Config file validation: Null byte check before filesystem ops, 1MB size limit, valibot schema validation
- Token escaping: Telegram bot token uses jsonEscape() (JSON.stringify()) before shell injection - correct
- Credentials in env vars: Follows spawn's existing pattern (TELEGRAM_BOT_TOKEN, GITHUB_TOKEN) - acceptable
- Input validation: Model ID and steps are validated downstream in orchestrate.ts - adequate
Tests
- bash -n: PASS (all .sh files have valid syntax)
- bun test: PASS (1405 tests pass, 0 fail, 3633 expect() calls)
- bunx @biomejs/biome lint: PASS (121 files checked, 0 errors)
- curl|bash: OK (no violations in shell scripts)
- macOS compat: OK (no bash 3.x incompatibilities)
Feature Validation
The PR adds two programmatic CLI flags:
- --config : Load options from JSON (with proper security validation)
- --steps : Control which setup steps run (github, browser, telegram, etc.)
Both features are well-tested (3 new test files with comprehensive coverage) and integrate cleanly with the existing architecture.
-- security/pr-reviewer
Summary
--config <path>flag to load spawn options from a JSON config file (model, steps, name, setup data)--steps <list>flag for comma-separated setup step controlTELEGRAM_BOT_TOKENenv var before falling back to interactive promptSPAWN_ENABLED_STEPSis already set (from--stepsor--config)spawn-config.tsmodule with valibot schema validationOptionalStepinterface extended withdataEnvVarandinteractivemetadatavalidateStepNames()validates step names and warns about unknowns.claude/rules/agent-setup-options.md) documenting all setup optionsConfig file format
{ "model": "openai/gpt-5.3-codex", "steps": ["github", "browser", "telegram"], "name": "my-dev-box", "setup": { "telegram_bot_token": "123456:ABC-DEF...", "github_token": "ghp_xxxx" } }Priority order (highest wins):
--model,--steps,--name)--configfileTest plan
bunx @biomejs/biome check src/— 0 errorsbun test— 1397 tests passbash -n sh/e2e/lib/verify.sh— syntax OKspawn codex gcp --config test.json --dry-runspawn openclaw gcp --steps github,browser --headless --output jsonTELEGRAM_BOT_TOKEN=xxx spawn openclaw gcp --steps telegram --headless🤖 Generated with Claude Code