Harden release floor, validate-first tools, and list pagination#23
Merged
Conversation
A zero or negative --min-tools value made the release tool-catalog floor a no-op. Parse the flag with a positive-integer argparse type, extract _build_parser() so CLI parsing is testable, and cover the parser and CLI accept/reject paths in unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AoFhWvujADVzy5GJq4aQ53
test_aaa constructed the Central client (triggering a live OAuth token refresh) before checking its required arguments, so invalid calls hit the network and the two invalid-input unit tests failed in any environment without credentials. Move get_client() below the validation branches so bad input returns early with no side effects, making `validate_release.py --skip-rag` pass hermetically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AoFhWvujADVzy5GJq4aQ53
Apply the same validate-first ordering as test_aaa to the remaining tools that constructed the Central client before rejecting invalid input: poe_bounce, port_bounce, cable_test, reboot_device, and create_webhook. Clients are now built only after validation (and after user confirmation for destructive ops), so rejected or declined calls have no credential/token side effects. Add regression tests for each rejected-input path asserting get_client is never called. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AoFhWvujADVzy5GJq4aQ53
Server-paged list tools re-sliced the returned page with the same offset, so any offset>0 page came back empty or wrong with a false truncated=False: passpoint profile/identity lists via _config_list_response, list_config_templates, and monitoring list_sites (under CENTRALMCP_BOUND_LISTS). They now slice from 0 and report the true offset, matching the existing list_devices pattern. Bound the two list tools that returned raw unbounded responses: list_config_assignments and list_firmware_upgrades gain limit/offset (and full_list for assignments) via bound_collection_response. Fix router convenience wrappers: list_sites/list_devices advertised a full_list parameter the backend tools don't have — FastMCP silently dropped it, returning a truncated page with no marker; the parameter is removed. list_scopes gains limit/offset/full_list forwarding so orgs with more scopes than one page can page past it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AoFhWvujADVzy5GJq4aQ53
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
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
Four related hardening fixes, each with regression tests:
1. Release floor can no longer be disabled (
scripts/validate_release.py)--min-toolsaccepted0/negatives, silently turning the tool-catalog floor into a no-op. A_positive_intargparse type now rejects non-integer and< 1values with exit code 2 (error: argument --min-tools: expected a positive integer, got '0')._build_parser()extracted frommain()so CLI parsing is unit-testable.2. Validate-first ordering (
mcp_servers/nac.py,ops.py,config.py)test_aaabuilt the Central client (triggering a live OAuth token refresh) before validating required args; invalid calls now return early with no side effects. This also made the release gate's unit-test step pass hermetically in credential-less environments.poe_bounce,port_bounce,cable_test,reboot_device,create_webhook. Clients are now built only after validation — and after user confirmation for destructive ops.3. Double-applied pagination offsets (
config.py,monitoring.py)offset>0page came back empty with a falsetruncated: False: passpoint profile/identity lists (_config_list_response),list_config_templates, andlist_sites(underCENTRALMCP_BOUND_LISTS). They now slice from 0 and report the true offset, matching the existinglist_devicespattern.4. Bounding-contract violations (
config.py,tool_router.py)list_config_assignmentsandlist_firmware_upgradesreturned raw unbounded responses; both now takelimit/offset(plusfull_listfor assignments) viabound_collection_response.list_sites/list_devicesadvertised afull_listparameter the backend tools don't have — FastMCP silently dropped it, returning a truncated page with no marker. Removed.list_scopeswrapper exposed no paging, so orgs with >100 scopes couldn't page past the first 100; it now forwardslimit/offset/full_list.No callers depended on the old behavior: nothing in the repo passes
--min-tools, and the phantomfull_listnever worked.Checklist
test_public_docs_tool_counts_match_catalog.)find_tool,invoke_read_tool,invoke_tool) where possible. (Not applicable — no user-facing examples changed.)test_list_output_bounding.pysuites).uv run python scripts/validate_release.py --skip-rag— 684 unit tests pass, RAG eval auto-skipped (local indexes absent), tool catalog floor satisfied (346 ≥ 204), exit 0.Security
For exploitable details or accidental credential exposure, do not use this pull request. Follow SECURITY.md.
Contributor notes
See CONTRIBUTING.md for local setup, validation, and no-secret contribution guidance.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AoFhWvujADVzy5GJq4aQ53