Skip to content

feat: fix escapeAssertion to preserve string literals #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
nomeguy merged 5 commits into from
Nov 11, 2025
Merged

feat: fix escapeAssertion to preserve string literals #514

nomeguy merged 5 commits into from
Nov 11, 2025

Conversation

Copy link
Contributor

Copilot AI commented Nov 11, 2025 •
edited
Loading

escapeAssertion was blindly replacing r. and p. patterns everywhere, including inside string literals. This broke policy rules containing strings like "r.my_resource", which became "r_my_resource".

// Before: fails because "r.my_resource" → "r_my_resource"
enforcer.addPolicy("alice", "r.my_resource", `p.obj == "r.my_resource"`);
await enforcer.enforce("alice", "r.my_resource"); // false ❌

// After: works correctly
await enforcer.enforce("alice", "r.my_resource"); // true ✅

Changes

src/util/util.ts

  • Replaced escapeAssertion implementation with Go Casbin's exact implementation
  • Uses regex /([()\s|&,=!><+\-*/]|^)((r|p)[0-9]*)\./g which naturally avoids string literals
  • Quotes (" and ') are not in the character class, so patterns inside strings are never matched
  • Preserved existing behavior for nested properties: r.obj.owner.id → r_obj.owner.id

Tests

  • Replaced test cases with Go Casbin's exact test suite from util_test.go
  • Includes all 20 test cases covering string literals, various operators, and edge cases
  • Added integration test reproducing the reported issue

Fixes #473

Original prompt

This section details on the original issue you should resolve

<issue_title>escapeAssertion does not respect string literals</issue_title>
<issue_description>Currently escapeAssertion does a rather brutal regex replace operation to convert r.*/p.* into r_*/p_* respectively. However it does not consider the case where the expression could contain a string literal. So if you have a rule that have some sort of string literal like p.obj == "r.something" it will fail in a very subtle manner.

const { newEnforcer, newModel } = require("casbin");

const MY_RESOURCE_NAME = "r.my_resource"; // change this to e.g. "a.my_resource" and it will work

(async function () {
  const model = newModel();
  model.addDef("r", "r", "act, obj");
  model.addDef("p", "p", "act, obj, rule");
  model.addDef("e", "e", "some(where (p.eft == allow))");
  model.addDef("m", "m", "r.act == p.act && r.obj == p.obj && eval(p.rule)");

  const enforcer = await newEnforcer(model);
  enforcer.addPolicy(
    "alice",
    MY_RESOURCE_NAME,
    `p.obj == "${MY_RESOURCE_NAME}"`
  );

  // does not work because internally it becomes `p_obj == "r_my_resource"`
  console.log(await enforcer.enforce("alice", MY_RESOURCE_NAME));
})();
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@hsluoyz</author><body>
@kevinresol can you see that if Go Casbin has already fixed this?</body></comment_new>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@CLAassistant
Copy link

CLAassistant commented Nov 11, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nomeguy nomeguy marked this pull request as ready for review November 11, 2025 17:03
Copilot AI changed the title [WIP] Fix escapeAssertion to handle string literals Fix escapeAssertion to preserve string literals Nov 11, 2025
Copilot AI requested a review from nomeguy November 11, 2025 17:19
@nomeguy
Copy link
Contributor

nomeguy commented Nov 11, 2025

@copilot

  • for escapeAssertion function, refer to Go Casbin's implementation exactly
  • for the new test cases, also use Go Casbin's implementation exactly
  • don't modify yarn.lock

Copy link
Contributor Author

Copilot AI commented Nov 11, 2025

@copilot

  • for escapeAssertion function, refer to Go Casbin's implementation exactly
    ...

Done in commit c67e086:

  • Replaced escapeAssertion with Go Casbin's exact implementation using regex /([()\s|&,=!><+\-*/]|^)((r|p)[0-9]*)\./g
  • Replaced test cases with Go Casbin's exact test suite from util_test.go
  • Reverted yarn.lock to original state

The Go implementation naturally handles string literals correctly because quotes are not in the regex character class, so patterns like "r. are never matched.

@nomeguy nomeguy changed the title Fix escapeAssertion to preserve string literals feat: fix escapeAssertion to preserve string literals Nov 11, 2025
@nomeguy nomeguy merged commit 9abab0f into master Nov 11, 2025
11 of 13 checks passed
hsluoyz pushed a commit that referenced this pull request Nov 11, 2025
@mserico
feat: fix escapeAssertion to preserve string literals (#514)
7801732
github-actions bot pushed a commit that referenced this pull request Dec 8, 2025
@semantic-release-bot
chore(release): 5.42.0 [skip ci]
368a15b 
# [5.42.0](v5.41.0...v5.42.0) (2025-12-08)

### Features

* configure CI workflow to trigger on master branch ([#521](#521)) ([02ec045](02ec045))
* fix duplicate CI runs on pull requests ([#513](#513)) ([67ab8ee](67ab8ee))
* fix escapeAssertion to preserve string literals ([#514](#514)) ([7801732](7801732))
* Support multiple policy definitions (p, p2) ([#518](#518)) ([1b1f1bb](1b1f1bb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nomeguy nomeguy Awaiting requested review from nomeguy

Assignees

@nomeguy nomeguy

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

escapeAssertion does not respect string literals

3 participants

@CLAassistant @nomeguy