fix(adt-mcp): fail closed on unbounded scoped-read dispatch - #223
Conversation
) isScopedReadResourceAllowed returned true for every scoped-read tool other than get_object/get_object_structure. Today the parser whitelist (scopedReadTools) makes that branch unreachable, but if the whitelist ever widened — e.g. to the cts_* transport reads — those tools would inherit unbounded access to arbitrary transport IDs, since resourceKeys only carry canonical TYPE:NAME object keys and cannot bind transports. Flip the default to deny: only resource-bound object reads dispatch under a scoped credential. Revive scope-enforcement.test.ts under vitest (it was dead code — excluded from the vitest include and unrunnable under node --test) and add a regression test proving a widened toolNames list still cannot dispatch cts_get_transport while a resource-bound get_object succeeds. Also update the stale ATC assertion: atc_run/run_unit_tests were reclassified as safe_execute in e0c792b, and the dormant test still asserted read. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (21)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughScoped read checks now allow only object lookup tools. Enforcement tests cover scoped access, and the ADT MCP tests switch to Vitest with an expanded test-file include pattern. ChangesScoped read authorization
Vitest test suite
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Scoped callers are denied transport reads while retaining access to in-scope objects. The change is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change strengthens scoped-read access: transport lookups are denied before their handlers run, while reads of an authorized object remain available. No introduced security finding was established. The assessment is limited to the examined dispatch path and does not cover every deployment or alternate registration path. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MergerReady to Merge The fail-closed dispatch change matches the stated security intent, and the only review concern was addressed by enabling the full Vitest suite. No unresolved blocker remains. |
codeant-ai
Bot
added
the
size:M
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The security fix properly implements fail-closed behavior for scoped-read dispatch. The change from return true to return false in isScopedReadResourceAllowed correctly prevents scoped-read credentials from reaching unbounded transport tools. Test coverage validates both the denial of cts_get_transport and the continued access to whitelisted object tools.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 5 |
| Duplication | -2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
✅ Deploy Preview for adt-cli canceled.
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
18 test files used node:test and were excluded from the vitest include, so they never ran in CI — authentication, safe-execution, HTTP, and registry regressions would pass undetected. Convert imports to vitest, map node:test before/after hooks to beforeAll/afterAll, and widen the include to tests/**/*.test.ts. Two stale assertions surfaced once the dormant tests ran: the ATC test (fixed in the previous commit — atc_run/run_unit_tests are safe_execute since e0c792b, not read) and a deepStrictEqual on verified claims whose null-prototype objects did not match plain-object literals (invocation intentionally clones untrusted claims into Object.create(null)). All 19 files / 208 tests pass. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
codeant-ai
Bot
added
size:L
|
Reconciled openspec/changes/ against shipped code (#224). Eight changes were fully implemented but never archived, leaving openspec/specs/ without an adt-mcp domain and invisible spec drift. Archived: - add-mcp-http-transport — spec delta converted from MODIFIED to ADDED (no adt-mcp spec existed to modify); 32 tasks ticked, 3 Docker artefact tasks left unchecked as deferred - add-delegated-assistant-read-scope — verified fail-closed dispatch - classify-atc-as-read-analysis — already reconciled to safe_execute - add-bounded-analysis-class — verification task ticked (PR #223 gates) - add-cts-transport-metadata-json — verification ticked; live-SAP proof deferred - add-flow-index-only, add-aclass-parser, arc-1-feature-parity add-aclass-parser shipped a prose spec without delta headers — the prose is preserved as design.md and rewritten as a proper ADDED delta. Remaining open changes are genuinely incomplete (live-SAP verification, credential rotation, or unfinished waves). Closes #224 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
User description
Summary
Fixes #222 — review findings from #221 (CodeAnt / CodeRabbit): read-scoped tokens must not reach CTS tools that accept arbitrary transport identifiers.
isScopedReadResourceAllowedinpackages/adt-mcp/src/lib/tools/scope-catalogue.tsnow fails closed: onlyget_object/get_object_structuredispatch under a scoped-read credential, and only when the call'sTYPE:NAMEobject key is inresourceKeys.scopedReadToolsinparseScopedAdtInvocationPolicyalready whitelists only the two object tools, so thereturn truefallback had no effect today — but any future widening of that whitelist (e.g. tocts_*reads) would have silently granted unbounded transport access.resourceKeysare canonical object keys (CLAS:ZCL_RELEASE_GATE); transport IDs likeDEVK900123have no binding form and are deliberately not reinterpreted. Transport-aware scoping needs a new contract + issuer change — out of scope here.Tests
tests/scope-enforcement.test.ts— it was dead code: excluded by the vitestincludeand unrunnable undernode --test(.js→.tsspecifiers). Convertednode:testimport →vitest, added it tovitest.config.ts. 12 tests now run innx test adt-mcp.toolNames: ['get_object', 'cts_get_transport']andresourceKeys: ['CLAS:ZCL_RELEASE_GATE']still getsmcp_scope_deniedoncts_get_transport(handler never called), whileget_objectfor the bound object succeeds.atc_run/run_unit_testswere reclassifiedread→safe_executein e0c792b; the dormant test still assertedreadand now asserts the correct class.Note: 17 other
tests/*.test.tsfiles in this package remain excluded from the vitest config and still usenode:test— same dead-code state this file was in. Worth a follow-up sweep.Test plan
bunx nx test adt-mcp— 106 tests pass (2 files)bunx nx lint adt-mcp— passbunx nx build adt-mcp— passbunx nx format:check— cleanGenerated with Devin
Summary by cubic
Fixes scoped-read dispatch so a read-scoped credential never reaches tools that accept arbitrary transport identifiers, even if the tool whitelist later widens.
isScopedReadResourceAllowednow denies every tool exceptget_object/get_object_structure; previously it returnedtruefor any other tool, an open default that is unreachable today but would grant unbounded transport access if the whitelist ever grew.Testing
tests/scope-enforcement.test.tsunder vitest with a regression test proving a widenedtoolNameslist still blockscts_get_transportwithmcp_scope_deniedwhile a resource-boundget_objectsucceeds.node:testto vitest and widensvitest.config.tsinclude totests/**/*.test.tsso they run in CI.atc_run/run_unit_testsare now asserted assafe_executeinstead ofread, and verified claims comparison accounts for null-prototype objects.Written for commit 7b41b2f. Summary will update on new commits.
CodeAnt-AI Description
Prevent scoped read access from reaching unbound transport tools
What Changed
Impact
✅ Prevented unauthorized transport reads✅ Preserved authorized object reads✅ Corrected ATC access control💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Bug Fixes
Tests