T09 · Insecure Skill Coding Practices
Warning
- Location
- src/event-bus.js:116
- Finding
- Object-Style Middleware Invocation Failure Causes Rate-Limit Bypass<![CDATA[ ## Vulnerability Details **File Location**: `src/event-bus.js:116-125` and `src/index.js:38-46` **Vulnerability Type**: Fail-open middleware integration and rate-limit bypass **Risk Level**: Medium ### Vulnerable Code `src/index.js:38-46` registers middleware as objects whose behavior is exposed through a `handle(event)` method: ```javascript _registerDefaultMiddleware() { // 日志中间件 this.eventBus.use(new LoggingMiddleware({ logLevel: 'info' })); // 速率限制中间件 this.eventBus.use(new RateLimitMiddleware({ maxEvents: 100, windowMs: 60000 })); } ``` However, `src/event-bus.js:116-125` invokes every registered middleware value as a function: ```javascript let shouldContinue = true; for (const middleware of this.middlewareChain) { try { const result = await middleware(event); if (result === false) { shouldContinue = false; break; } } catch (error) { console.error(`Middleware error: ${error.message}`); } } ``` ### Technical Analysis `LoggingMiddleware` and `RateLimitMiddleware` are class instances that expose an asynchronous `handle(event)` method. They are not callable JavaScript functions. Consequently, the expression `middleware(event)` throws a `TypeError` whenever one of these default middleware objects is processed. The exception is caught and only logged. Event processing then continues with the next middleware and ultimately reaches event history and subscribers. This fail-open behavior means the default rate limiter never counts or blocks events, despite being enabled by the orchestrator constructor. The standalone `MiddlewareChainExecutor` elsewhere in the project correctly distinguishes between object-style and function-style middleware, but `EventBus.publish()` does not implement the same dispatch logic. Tests exercise middleware classes directly without confirming that object-style middleware is invoked through the integrated `EventOrchestrator.publish()` path. ### Attack Path 1. An atta ...[truncated 1575 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Support the documented object-style middleware interface explicitly: ```javascript for (const middleware of this.middlewareChain) { let result; if (middleware && typeof middleware.handle === 'function') { result = await middleware.handle(event); } else if (typeof middleware === 'function') { result = await middleware(event); } else { throw new TypeError('Middleware must be a function or expose handle(event)'); } if (result === false) { return { eventId, status: 'skipped', reason: 'middleware_blocked' }; } } ``` 2. Validate middleware in `EventBus.use()` so malformed middleware is rejected at registration time rather than failing during publication: ```javascript use(middleware) { const valid = typeof middleware === 'function' || (middleware && typeof middleware.handle === 'function'); if (!valid) { throw new TypeError('Invalid middleware'); } this.middlewareChain.push(middleware); } ``` 3. Define an explicit failure policy. Security controls such as rate limiting and validation should fail closed rather than allowing publication after an internal middleware error. 4. Avoid swallowing middleware exceptions without classification. Return a failed publication result or propagate the exception when a mandatory middleware component fails. 5. Add integration tests through `EventOrchestrator.publish()` that verify: - Object-style middleware has its `handle()` method invoked. - The 101st same-name event within 60 seconds is blocked by default. - A middleware result of `false` prevents history insertion and subscriber dispatch. - Invalid middleware is rejected during registration. - Mandatory middleware exceptions do not permit event delivery. ]]>