Skip to content

fix(commands): close requires_auth() fail-open gap in 19 CommandHandler impls#6033

Merged
bug-ops merged 1 commit into
mainfrom
fix/6003-commandhandler-requires-auth
Jul 11, 2026
Merged

fix(commands): close requires_auth() fail-open gap in 19 CommandHandler impls#6033
bug-ops merged 1 commit into
mainfrom
fix/6003-commandhandler-requires-auth

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

Test plan

  • cargo nextest run --config-file .github/nextest.toml -p zeph-commands --all-features — 177/177 passed, including one dispatch-level allowed/rejected test pair per gated handler (or direct unit assert for acp.rs/cocoon.rs, matching each file's existing test convention).
  • cargo +nightly fmt --check — clean.
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings — clean.
  • RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --all-features -p zeph-commands and cargo test --doc -p zeph-commands --all-features — clean.
  • Independent adversarial critique (impl-critic): re-verified sweep completeness (all 50 handlers enumerated), trust-gate propagation across both dispatch paths, test-assertion quality (keys on the gate-exclusive error string, no false positives), scope-boundary correctness for the 6 ungated handlers, and feature-gate compile correctness under single-feature builds (--features cocoon). Verdict: approved, no blocking findings.
  • Independent code review: approved, no blocking findings.
  • Live untrusted-channel rejection (Telegram/Discord/Slack) not exercised — same limitation as the two prior fixes in this class (requires live bot credentials, unavailable in CI). Tracked in .local/testing/coverage-status.md.

…er impls

CommandHandler::requires_auth() defaults to false, so any handler that
omits an explicit override runs even from untrusted remote channels
(Telegram/Discord/Slack). This is the third confirmed instance of the
defect class, after /image (#5967) and /mcp+/plugins (#5997).

Audited all 50 CommandHandler impls in crates/zeph-commands/src/handlers/
and added requires_auth() -> true to the 19 that mutate local filesystem,
process, or config state, or disclose sensitive configuration: UndoCommand,
RedoCommand, PlanCommand, SkillCommand, KnowledgeSlashCommand, AgentCommand,
AgentsFleetCommand, ConvCommand, MemoryCommand, GraphCommand, GoalCommand,
AcpCommand, CompactCommand, FeedbackCommand, CocoonCommand,
NewConversationCommand, ClearCommand, ResetCommand, ClearQueueCommand.

RecapCommand, SkillsCommand, GuidelinesCommand, ExitCommand, QuitCommand,
and HelpCommand were audited and confirmed safe as read-only or already
self-gated, and left unchanged. The trait's fail-open default is
unchanged; flipping it to fail-closed is deferred as a separate
architectural decision.

Closes #6003
@github-actions github-actions Bot added documentation Improvements or additions to documentation rust Rust code changes bug Something isn't working size/XL Extra large PR (500+ lines) labels Jul 11, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 11, 2026 14:03
@bug-ops
bug-ops merged commit f3d6300 into main Jul 11, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/6003-commandhandler-requires-auth branch July 11, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation rust Rust code changes size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

audit sibling CommandHandler impls for the same fail-open requires_auth gap (checkpoint.rs, cocoon.rs, and others)

1 participant