fix: batch fix for cortex-cli bugs (exit codes, conflicting flags, JSON output, security)#564
Open
fix: batch fix for cortex-cli bugs (exit codes, conflicting flags, JSON output, security)#564
Conversation
Replace emoji characters with ASCII text alternatives: - 🔀 → [PR] - ⏳ → [WAIT] Fixes #3792
…nd jobs=0 - Issue #3815: Reject duplicate task IDs in DAG specification - Issue #3722: Validate --sort flag values in models list command - Issue #3716: Reject --jobs 0 which would cause hang
SECURITY FIX: The allows_risk() function was incorrectly passing the risk
level string to is_read_only_command() instead of the actual command.
This bug meant that in read-only mode, is_read_only_command("low") was
always returning false (since "low" doesn't match any read-only patterns),
which could allow non-read-only commands to be auto-approved.
Changes:
- Added 'command' parameter to allows_risk() function
- Fixed the call to is_read_only_command(command) instead of (risk)
- Added comprehensive tests for allows_risk() function
Fixes #3851
- file.rs: run_file now returns error when file doesn't exist (#3896) - handlers.rs: run_whoami now returns Result<()> with proper error (#3891) - handlers.rs: config get/unset now return errors on key not found (#3820) - upgrade_cmd.rs: invalid channel now returns error instead of Ok(()) (#3843)
- exec_cmd: Check --enabled-tools and --disabled-tools are not both specified - mcp_cmd/auth: Check --name and --all are not both specified for logout - agent_cmd: Check --primary and --subagents are not both specified for list - plugin_cmd: Check --enabled and --disabled are not both specified for list - mcp_cmd/handlers: Check --url and --sse are not both specified for add - cli/handlers: Check only one auth method (--token, --sso, --device-auth, --with-api-key) is specified for login Issues: #3885, #3857, #3856, #3855, #3819, #3814
- config.rs: Pure JSON output when --json with --diff (Issue #3879) - compact_cmd.rs: JSON output for dry-run with --json (Issue #3826) - alias_cmd.rs: JSON error when alias not found with --json (Issue #3735) - plugin_cmd.rs: JSON error when plugin not installed with --json (Issue #3723)
- Fix debug config to detect config.json instead of only config.toml (#3150) - Fix uninstall --backup to create backup after confirmation, not before (#3682) - Fix plugin install to validate empty plugin names (#3700) - Fix lock add to validate session ID format (#3696) - Fix resume command to accept 'last' as SESSION_ID per help text (#3646) - Fix help text to reference correct GitHub repo URL (#3678) - Fix upgrade --changelog to fetch raw content instead of HTML (#3651)
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
This PR addresses multiple open issues from the PlatformNetwork/bounty-challenge repository affecting cortex-cli.
Issues Fixed
Exit Code Issues
cortex debug filenow returns non-zero exit code when file does not existwhoamicommand now returns error when checking login status failsupgradecommand now returns error for invalid channel (stable/beta/nightly)config get/unsetnow return error when key not foundConflicting Flags Validation
execcommand validates that--enabled-toolsand--disabled-toolscannot both be specifiedmcp logoutvalidates that--nameand--allcannot both be specifiedagent listvalidates that--primaryand--subagentscannot both be specifiedplugin listvalidates that--enabledand--disabledcannot both be specifiedmcp addvalidates that--urland--ssecannot both be specifiedloginvalidates that only one auth method can be specifiedJSON Output Issues
debug config --diff --jsonnow outputs pure JSONdag run --strategy dry-runrespects--formatflagcompact run --dry-run --jsonoutputs JSONdag list --format jsonoutputs proper JSONalias show --jsonoutputs JSON on errorsplugin show --jsonoutputs JSON on errorsdebug wait --jsonoutputs JSON on errorsmcp get --jsonoutputs JSON on errorsrun --format jsonoutputs JSON on auth errorsSecurity Fix
allows_risk()to pass actual command tois_read_only_command()instead of risk levelInput Validation Issues
models list --sortvalidates sort values (name, provider, context, created, id)dag run --jobs 0now errors instead of hangingplugin install ""now returns proper error for empty namelock addnow validates session ID format (UUID or 8-char prefix)User Experience Fixes
resumecommand now accepts 'last' as SESSION_ID per help textupgrade --changelognow fetches raw content instead of HTMLdebug confignow correctly detects config.json filesuninstall --backupnow creates backup after user confirmationprcommand output (replaced with text alternatives)Changes
bail!()macro--jsonflag output valid JSON even on errorsTesting
cargo check -p cortex-clipassescargo fmt --allappliedNote
This PR is for review only - DO NOT MERGE until reviewed.