fix: CLI, config, credentials hardening (slice 11)#124
Merged
Conversation
- ToolsConfig defaults: planning=[Read,Glob,Grep,WebFetch,WebSearch], verification=[Read,Glob,Grep,Bash], working=[] (all tools) - Planner can no longer Write/Edit/Bash before a plan exists, matching CLAUDE.md's phase table (empty list still = all tools) - info.py status now derives planning tool display from config instead of hardcoding a stale "Read, Glob, Grep, Bash" string - Sync docstrings/example JSON; update config/agent/info tests Co-Authored-By: Claude <noreply@anthropic.com>
start/resume never acquired the single-instance session lock (only merge-pr and clean did), so two concurrent runs could corrupt state.json, duplicate PRs, and race OAuth refresh-token rotation. - start: acquire after exists(), before credential load — closes the OAuth-rotation window; release in finally. - resume: acquire after validate_for_resume so a no-op resume of an already-finished task exits cleanly without taking the lock; release in finally. - Release is idempotent with the orchestrator's cleanup_on_success and only removes a lock this PID owns. - Add tests/cli/test_session_lock.py (acquire guard + release on every exit path, for both commands). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
config show dumped ANTHROPIC_API_KEY/OPENROUTER_API_KEY in plaintext (env overrides merge into the config), and start accepted --max-sessions 0 (silently = unlimited), negatives, --prs 0, --budget 0, and empty goals. - config show: recursively mask secret-looking leaves (key/secret/token/ password) in both formatted and --raw output; --show-secrets opt-in to reveal; masking hint shown when anything is redacted. --env honors --show-secrets and reuses the shared _mask_secret helper. - start: min=1 on --max-sessions/--prs (parse-time IntRange), _validate_budget callback rejects <=0, _validate_goal callback rejects empty/whitespace goals. All rejected at parse time before any state/credential work. - Tests: config-show masking + secret helpers; start input-validation rejections and callback unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- debug_claude_md.py: Use get_config().models.haiku instead of hardcoded model id; reuse CredentialManager for credentials check - doctor.py: Use CredentialManager().verify_credentials() with profile awareness instead of raw path check - profiles.py: Add force parameter to remove() method; warn/refuse removing active profile without --force flag - credentials.py: Raise InvalidCredentialsError on empty api-key for api-key profiles (both get_valid_token and verify_credentials) - cli_commands/profile.py: Pass force flag to manager.remove() - cli_commands/mailbox.py: Register original mailbox_send/mailbox_clear as commands; remove duplicate wrapper functions - tests: Update tests to mock CredentialManager and verify new profile removal behavior Fixes: ✓ debug_claude_md uses config-driven model selection ✓ doctor command profile-aware via CredentialManager ✓ profile removal protects active profile by default ✓ credentials properly validate empty api-keys ✓ mailbox commands consolidated (no duplicate defs) All 127 tests pass; ruff/mypy checks pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- tests/cli_commands/test_profile.py: 37 tests covering all 6 profile commands (_mask helper, add/list/use/show/remove/login) plus register_profile_commands — was zero coverage before. - tests/core/test_agent_phases.py: add end-to-end chain test verifying that run_planning_phase() passes the config-driven restricted tool list (["Read","Glob","Grep","WebFetch","WebSearch"]) to query_executor.run_query, so destructive tools (Bash/Edit/Write) never reach ClaudeAgentOptions.allowed_tools during planning. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Slice 11 moved _check_credentials to profile-aware CredentialManager().verify_credentials() (validates content), but 3 tests still patched Path.home + wrote real files, so they resolved the runner's real ~/.claude and failed in CI. - test_all_checks_pass: mock CredentialManager (matches siblings) - test_credentials_path_with_special_characters: drive real CredentialManager via config_dir on a spaced path with valid creds - test_empty_credentials_file: empty file is now invalid JSON, so the check correctly fails (content validation is done) Co-Authored-By: Claude <noreply@anthropic.com>
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
config showrecursively masks key/secret/token/password fields by default;--show-secretsopts in--force; doctor is profile-aware; api-key profiles reject empty keystests/cli_commands/test_profile.py(37 tests, was zero); end-to-end chain test verifying planning tools reachClaudeAgentOptions.allowed_toolsTest plan
pytest tests/cli_commands/test_profile.py— 37 new profile command testspytest tests/core/test_agent_phases.py— includes new planning-tools chain testpytest tests/cli_commands/test_workflow.py— validation rejection testspytest tests/cli/test_config_commands.py— config-show masking testspytest tests/cli/test_session_lock.py— concurrent-start lock testspytest tests/ --no-cov→ 5383 passed🤖 Generated with Claude Code