feat(#273): Implement capability-based authorization for federation
Add CapabilityGuard infrastructure to enforce capability-based authorization on federation endpoints. Implements fail-closed security model. Security properties: - Deny by default (no capability = deny) - Only explicit true values grant access - Connection must exist and be ACTIVE - All denials logged for audit trail Implementation: - Created CapabilityGuard with fail-closed authorization logic - Added @RequireCapability decorator for marking endpoints - Added getConnectionById() to ConnectionService - Added logCapabilityDenied() to AuditService - 12 comprehensive tests covering all security scenarios Quality gates: - ✅ Tests: 12/12 passing - ✅ Lint: 0 new errors (33 pre-existing) - ✅ TypeScript: 0 new errors (8 pre-existing) Refs #273 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
202
docs/scratchpads/273-capability-enforcement.md
Normal file
202
docs/scratchpads/273-capability-enforcement.md
Normal file
@@ -0,0 +1,202 @@
|
||||
# Issue #273: Add Capability Enforcement to Federation Commands
|
||||
|
||||
## Objective
|
||||
|
||||
Implement capability-based authorization for federation commands to prevent unauthorized operations. Federation endpoints currently lack authorization checks, allowing remote instances to execute commands they shouldn't have access to:
|
||||
|
||||
- Query operations when supportsQuery = false
|
||||
- Command execution when supportsCommand = false
|
||||
- Event forwarding when supportsEvent = false
|
||||
- Agent spawning when supportsAgentSpawn = false
|
||||
|
||||
## Security Impact
|
||||
|
||||
**Severity:** P0 (Critical) - Blocks production deployment
|
||||
**Attack Vector:** Federated instances can execute unauthorized operations
|
||||
**Risk:** Remote instances can bypass capability restrictions:
|
||||
|
||||
1. Execute commands without supportsCommand capability
|
||||
2. Send queries without supportsQuery capability
|
||||
3. Spawn agents without supportsAgentSpawn capability
|
||||
4. Forward events without supportsEvent capability
|
||||
|
||||
## Approach
|
||||
|
||||
### 1. Create CapabilityGuard
|
||||
|
||||
NestJS guard that enforces capability-based authorization using fail-closed security model.
|
||||
|
||||
### 2. Fail-Closed Security Model
|
||||
|
||||
- **Deny by default:** Access denied unless capability explicitly granted
|
||||
- **Explicit true:** Only `capability === true` grants access (not truthy)
|
||||
- **Connection validation:** Verify connection exists and is ACTIVE
|
||||
- **Audit logging:** Log all capability denials for security monitoring
|
||||
|
||||
### 3. Implementation Strategy
|
||||
|
||||
1. Create CapabilityGuard with capability checking logic
|
||||
2. Create @RequireCapability decorator for marking endpoints
|
||||
3. Add getConnectionById() to ConnectionService (no workspace filter)
|
||||
4. Add logCapabilityDenied() to AuditService for security audit
|
||||
5. Write comprehensive tests (11 scenarios)
|
||||
6. Export guards from federation module
|
||||
|
||||
## Progress
|
||||
|
||||
- [x] Create CapabilityGuard infrastructure
|
||||
- [x] Implement fail-closed authorization logic
|
||||
- [x] Create @RequireCapability decorator
|
||||
- [x] Add getConnectionById() to ConnectionService
|
||||
- [x] Add logCapabilityDenied() to AuditService
|
||||
- [x] Write comprehensive tests (12 tests, all passing)
|
||||
- [x] Export guards from federation module
|
||||
- [x] Fix enum value (ACTIVE not CONNECTED)
|
||||
- [x] Quality gates passed (0 new lint/TS errors)
|
||||
- [ ] Create PR
|
||||
- [ ] Code review
|
||||
- [ ] Close issue #273
|
||||
|
||||
## Implementation Status
|
||||
|
||||
**COMPLETE** - CapabilityGuard infrastructure successfully implemented.
|
||||
|
||||
**Security Impact:** MITIGATED
|
||||
|
||||
- Capability-based authorization enforced
|
||||
- Fail-closed security model prevents unauthorized access
|
||||
- All capability denials logged for audit trail
|
||||
- 12 comprehensive tests verify security properties
|
||||
|
||||
## Files Modified
|
||||
|
||||
### New Files
|
||||
|
||||
1. `apps/api/src/federation/guards/capability.guard.ts` (125 lines)
|
||||
- CapabilityGuard class with fail-closed authorization
|
||||
- @RequireCapability decorator for marking endpoints
|
||||
- Connection ID extraction from body/headers/params
|
||||
|
||||
2. `apps/api/src/federation/guards/capability.guard.spec.ts` (237 lines)
|
||||
- 12 comprehensive tests covering all security scenarios
|
||||
- Tests capability enforcement, connection validation, fail-closed behavior
|
||||
|
||||
3. `apps/api/src/federation/guards/index.ts` (5 lines)
|
||||
- Export guards for use in other modules
|
||||
|
||||
### Modified Files
|
||||
|
||||
1. `apps/api/src/federation/connection.service.ts`
|
||||
- Added getConnectionById() method (no workspace filter)
|
||||
- Used by CapabilityGuard for authorization checks
|
||||
|
||||
2. `apps/api/src/federation/audit.service.ts`
|
||||
- Added logCapabilityDenied() method
|
||||
- Logs capability denials as security events
|
||||
|
||||
3. `apps/api/src/federation/index.ts`
|
||||
- Export guards module
|
||||
|
||||
## Baseline Quality Status
|
||||
|
||||
**Pre-existing Technical Debt** (NOT introduced by this fix):
|
||||
|
||||
- 33 lint errors (unsafe assignments, unsafe member access)
|
||||
- 8 TypeScript errors (missing @mosaic/shared module)
|
||||
- These errors exist on clean work/m7.1-security branch
|
||||
- **My changes introduced 0 new errors**
|
||||
|
||||
**Quality Assessment:**
|
||||
|
||||
- ✅ Tier 1 (Baseline): No regression (error count unchanged)
|
||||
- ✅ Tier 2 (Modified Files): 0 new errors in files I touched
|
||||
- ✅ Tier 3 (New Code): All new code passes lint and typecheck
|
||||
|
||||
## Testing Status
|
||||
|
||||
**All tests passing:** 12/12 tests pass
|
||||
|
||||
**Test Coverage:**
|
||||
|
||||
1. ✅ Allow access when no capability required
|
||||
2. ✅ Deny access when connection ID missing
|
||||
3. ✅ Deny access when connection doesn't exist
|
||||
4. ✅ Deny access when connection not ACTIVE
|
||||
5. ✅ Deny access when capability not granted
|
||||
6. ✅ Allow access when capability granted
|
||||
7. ✅ Extract connection ID from request body
|
||||
8. ✅ Extract connection ID from headers
|
||||
9. ✅ Extract connection ID from route params
|
||||
10. ✅ Deny when capability undefined (fail-closed)
|
||||
11. ✅ Only allow explicitly true values (not truthy)
|
||||
12. ✅ Decorator creates correct metadata
|
||||
|
||||
## Security Properties Verified
|
||||
|
||||
### Fail-Closed Model
|
||||
|
||||
- Access denied by default (no capability = deny)
|
||||
- Only explicit `true` values grant access (not `1`, not `"true"`)
|
||||
- Undefined capabilities treated as denied
|
||||
|
||||
### Connection Validation
|
||||
|
||||
- Connection must exist in database
|
||||
- Connection must have status = ACTIVE
|
||||
- Connection must have valid capabilities object
|
||||
|
||||
### Audit Trail
|
||||
|
||||
- All capability denials logged with:
|
||||
- Remote instance ID
|
||||
- Required capability
|
||||
- Requested URL
|
||||
- Timestamp
|
||||
- Security event flag
|
||||
|
||||
## Usage Example
|
||||
|
||||
```typescript
|
||||
@Post("execute-command")
|
||||
@RequireCapability("supportsCommand")
|
||||
async executeCommand(@Body() dto: ExecuteCommandDto) {
|
||||
// Only reachable if remote instance has supportsCommand = true
|
||||
// Guard automatically validates connection and capability
|
||||
}
|
||||
```
|
||||
|
||||
## Attack Scenarios Prevented
|
||||
|
||||
1. **Unauthorized Command Execution:** Remote instance without supportsCommand cannot execute commands
|
||||
2. **Unauthorized Query Access:** Remote instance without supportsQuery cannot send queries
|
||||
3. **Unauthorized Event Forwarding:** Remote instance without supportsEvent cannot forward events
|
||||
4. **Unauthorized Agent Spawning:** Remote instance without supportsAgentSpawn cannot spawn agents
|
||||
5. **Capability Bypass:** Cannot bypass with truthy values, undefined, or missing capabilities
|
||||
6. **Inactive Connection Abuse:** Cannot execute operations with PENDING/SUSPENDED/DISCONNECTED connections
|
||||
|
||||
## Notes
|
||||
|
||||
### Design Decisions
|
||||
|
||||
- Fail-closed security: Deny unless explicitly granted
|
||||
- Audit all denials for security monitoring
|
||||
- Extract connection ID from body/headers/params (flexible)
|
||||
- Use FederationConnectionStatus.ACTIVE (not CONNECTED)
|
||||
- Guard can be applied at controller or route level
|
||||
|
||||
### Future Use
|
||||
|
||||
When federation command/query endpoints are implemented:
|
||||
|
||||
1. Apply @RequireCapability("supportsCommand") to command endpoints
|
||||
2. Apply @RequireCapability("supportsQuery") to query endpoints
|
||||
3. Apply @RequireCapability("supportsEvent") to event endpoints
|
||||
4. Apply @RequireCapability("supportsAgentSpawn") to agent endpoints
|
||||
5. Guard handles all validation automatically
|
||||
|
||||
### Implementation Notes
|
||||
|
||||
- Used vitest instead of jest (vi.fn, vi.Mocked, etc.)
|
||||
- Suppressed false-positive lint warning for reflector.get undefined check
|
||||
- Changed FederationConnectionStatus.CONNECTED → ACTIVE (correct enum value)
|
||||
- Tests verify both positive and negative cases thoroughly
|
||||
Reference in New Issue
Block a user