Conversation
Add withRequiredMcpScope support in @dkg/plugins and apply it to EPCIS MCP handlers while preserving the standard mcp.registerTool flow. Expand integration tests and documentation to cover MCP transport scope plus tool-level epcis.read/epcis.write requirements. Made-with: Cursor
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds EPCIS scope enforcement across API and MCP, plus new integration coverage, but there is a security-critical fail-open path in the new MCP scope guard. The test suite moves in the right direction, yet key newly-guarded read paths are still untested and a couple of assertions are too weak to catch real regressions. Maintainability in the touched auth boundary is somewhat worsened by duplicating EPCIS route-scope mapping outside the plugin, which increases drift risk.
| }, | ||
| }, | ||
| async (input) => { | ||
| withRequiredMcpScope("epcis.read", async (input) => { |
There was a problem hiding this comment.
🔴 Bug: This introduces a new auth boundary for epcis-track-item, but the new integration tests do not verify its scope behavior (they only exercise epcis-query, epcis-capture, and epcis-capture-status). Add regression tests for epcis-track-item with mcp-only, read-only, and write-only tokens to prevent silent authorization regressions.
apps/agent/tests/integration/workflows/epcis-authorization.spec.ts
Outdated
Show resolved
Hide resolved
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
This PR adds EPCIS scope enforcement primitives and a strong integration test matrix for read/write authorization across HTTP and MCP paths. The extraction of route constants and reusable scope-guard helpers improves maintainability in the touched EPCIS/plugin areas. I did not find a clear runtime blocker in the changed auth logic, but there are two reliability/maintainability gaps in the integration test setup that should be fixed before merge. Overall direction is good, with minor cleanup needed to keep tests deterministic and dependency wiring explicit.
| import { authorized, createOAuthPlugin } from "@dkg/plugin-oauth"; | ||
| import dkgEssentialsPlugin from "@dkg/plugin-dkg-essentials"; | ||
| import createFsBlobStorage from "@dkg/plugin-dkg-essentials/createFsBlobStorage"; | ||
| import epcisPlugin, { applyEpcisHttpScopeGuards } from "@dkg/plugin-epcis"; |
There was a problem hiding this comment.
🟡 Issue: @dkg/plugin-epcis is imported here but not declared in apps/agent/package.json; add it as a dependency/devDependency so this workspace can run tests reliably in isolated installs and keeps the dependency graph explicit.
|
|
||
| const actualPort = (server.address() as any)?.port || port; | ||
| const url = `http://localhost:${actualPort}`; | ||
| process.env.EXPO_PUBLIC_MCP_URL = url; |
There was a problem hiding this comment.
🟡 Issue: This mutates process.env.EXPO_PUBLIC_MCP_URL globally without restoring it in cleanup, which can make later tests order-dependent. Save the previous value before setting it and restore/delete it in the returned cleanup function.
1 participant
Add withRequiredMcpScope support in @dkg/plugins and apply it to EPCIS MCP handlers while preserving the standard mcp.registerTool flow. Expand integration tests and documentation to cover MCP transport scope plus tool-level epcis.read/epcis.write requirements.