fix(authz): make Cedar URI entity IDs collision-free

[SashaMIT]

A permission granted to one resource URI can silently apply to a different, colliding URI.

Problem

authorizeResourceRead turned resource URIs into Cedar entity IDs with a lossy sanitizer that rewrote :, /, \, ?, &, =, #, space, and . to _. The mapping is many-to-one, so distinct URIs collide onto one entity ID:

• file:///etc/passwd and file://_etc/passwd both became Resource::"file____etc_passwd"
• mcp://srv/config:admin and mcp://srv/config/admin both became Resource::"mcp___srv_config_admin"

A policy granting Resource::"file____etc_passwd" therefore authorized reads of every URI in the collision class. The confused-deputy condition: an MCP server that serves both a permitted URI and a colliding sensitive URI (or one that lets users create resource URIs) gets the grant applied to a resource the policy author never named.

Severity framing, honestly stated: exploitation needs a policy author who grants by entity ID, plus a colliding pair where one side is permitted and the other is sensitive and server-distinguished. Attribute-based policies (resource.name == ..., resource.uri == ...) match on the exact URI and are unaffected.

Fix

Entities are built programmatically with cedar.NewEntityUID, which accepts any string, so no character rewriting is needed at all. The exact URI is now the entity ID and the sanitizer is removed. Policy authors can name the real URI (for example Resource::"file:///etc/passwd") instead of computing a mangled form.

Note for existing policies: a policy that references a sanitized ID (only possible for URIs containing the rewritten characters) must be updated to name the exact URI. Policies using plain IDs or attribute matching are unchanged.

Tests

Replaced TestSanitizeURIForCedar with TestAuthorizeResourceReadEntityIDsCollisionFree: for both collision pairs above, a grant on the exact URI authorizes that URI and denies the formerly colliding URI. The pkg/authz/... and pkg/vmcp/core suites pass.

Made with Cursor

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.82%. Comparing base (c86cb8a) to head (325d907).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6239      +/-   ##
==========================================
- Coverage   72.83%   72.82%   -0.01%     
==========================================
  Files         743      743              
  Lines       77649    77635      -14     
==========================================
- Hits        56556    56541      -15     
- Misses      17125    17129       +4     
+ Partials     3968     3965       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jhrozek]

The fix looks right to me. I traced both sides: the request UID comes from cedar.NewEntityUID (core.go:513-516) and the entity-map key comes from the same parseCedarEntityID split (entity.go:152,187), so the two stay in sync and arbitrary URI characters are safe. parseCedarEntityID uses SplitN(..., "::", 2), so a URI containing "::" still parses with the full URI as the ID. And since every resource-read path (middleware.go:49-53, response_filter.go:781, vmcp/core/admission.go:199,218) funnels through authorizeResourceRead, this fixes it in one place rather than per caller.

Two comments below, both about documenting the new form rather than the code itself. Non-blocking.

[jhrozek]

Could we get a line in docs/authz.md to go with this? The comment explains the why nicely, but nothing user-facing says what a resource policy should now look like. The Resource examples in docs/authz.md are all Resource::"data" — a name with no special characters — so someone writing a per-URI policy has no way to know the right form is Resource::"file:///etc/passwd".

Worth doing even though I doubt anyone was relying on the old sanitized IDs. The sanitization was never documented, so to write Resource::"file____etc_passwd" you'd have had to turn on debug logging, read the entity ID off the cedar authorization check line, and copy it back into your policy. The more likely experience is the one in pkg/vmcp/core/core_calls_test.go:718, where the policy is written as Resource::"file:///ok" — the intuitive exact-URI form, which silently matched nothing under the old code. That test still passed because it only asserts the deny half. So the docs line is mostly for the next person who tries the obvious thing.

[jhrozek]

Small caveat on "accepts any string": that's true of the request side, but a policy still has to name the entity in Cedar source, where the ID is a double-quoted string literal. For almost every URI that's invisible — : / ? & = # . are all ordinary inside a Cedar string — but " and \ need escaping:

permit(principal, action == Action::"read_resource",
       resource == Resource::"file://C:\\share\\data");

The old sanitizer folded \ to _, so this is newly visible to policy authors. Not a reason to change anything here (a lossy mapping was strictly worse), but the same docs line from the other comment could mention it.

[SashaMIT]

Thanks @jhrozek. Added a docs/authz.md note that read_resource entity IDs are the exact URI (with a Resource::"file:///..." example), plus the Cedar string-literal escape caveat for " / \. Softened the code comment to match. Happy to tweak the wording if you want it shorter.