feat(policy): add one-time MCP approvals - #65
Conversation
Implement connection-bound approvals with native elicitation, safe fallback bearers, lifecycle audit records, expiry/replay protection, and serialized audit-safe transitions.
📝 WalkthroughWalkthroughThis change adds connection-bound, one-time approvals for confirmation-required MCP operations, including fallback tokens, form elicitation, approval management tools, expiry and replay protection, audited lifecycle transitions, bounded in-memory storage, and integration tests. ChangesApproval lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MiftahServer
participant ApprovalStore
participant OperationPipeline
participant Upstream
Client->>MiftahServer: Call confirmation-required operation
MiftahServer->>OperationPipeline: Execute with request context
OperationPipeline->>MiftahServer: Request approval for exact binding
MiftahServer->>ApprovalStore: Create pending approval
MiftahServer-->>Client: Form elicitation or fallback token flow
Client->>MiftahServer: Approve or deny approval
MiftahServer->>ApprovalStore: Consume approved binding once
MiftahServer->>OperationPipeline: Continue approved operation
OperationPipeline->>Upstream: Forward exact operation
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp/server/miftah-server.ts`:
- Around line 935-953: Update withApprovalExpiryAudit so failures from
writeExpiredApprovalTransitions in the operation() catch branch cannot replace
the original operation error. Attempt the audit write while suppressing or
separately handling its failure, then rethrow the original caught error; keep
the existing rollback behavior for audit failure after a successful operation.
- Around line 976-991: Update the writeApproval method to use the existing
ApprovalAuditAction type from audit-types.ts (or the established audit-trail
re-export) instead of its inline literal union, preserving the current allowed
action values while eliminating the duplicate source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0843890a-bd51-48d6-b4b9-fed83d66bb34
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mddocs/architecture.mddocs/config.mddocs/security.mdsrc/approvals/approval-store.tssrc/audit/audit-trail.tssrc/audit/audit-types.tssrc/cli/exit-codes.tssrc/mcp/server/miftah-server.tssrc/mcp/server/operation-pipeline.tssrc/utils/errors.tstests/approval-docs-contract.test.tstests/approval-fallback.test.tstests/approval-store.test.tstests/cli-exit-codes.test.tstests/operation-pipeline.test.ts
Closes #27.
Summary
Implements connection-bound, one-time approvals for confirmation-required tools, resources, and prompts:
Security impact
Approval bearers and raw operation arguments are never persisted or audited. Stored state contains only keyed digests and safe metadata; a bearer can authorize only its exact, connection-bound operation once. Failed expiry audit writes retain the transition for retry and expose audit health without replacing the caller's actionable approval error.
Validation
npm run lintandnpm run typecheckpass