Skip to content

Commit 7801732

Browse files
committed
feat: fix escapeAssertion to preserve string literals (#514)
1 parent 67ab8ee commit 7801732

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

src/util/util.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@
1717

1818
import { mustGetDefaultFileSystem } from '../persist';
1919

20-
const escapeAssertionReg = new RegExp(/(^|[^A-Za-z0-9_])([rp])[0-9]*\./g);
20+
const escapeAssertionReg = new RegExp(/([()\s|&,=!><+\-*/]|^)((r|p)[0-9]*)\./g);
2121

2222
function escapeAssertion(s: string): string {
23-
s = s.replace(escapeAssertionReg, (match, p1, p2) => {
24-
return p1 + p2 + match.substring(p1.length + p2.length).replace('.', '_');
23+
s = s.replace(escapeAssertionReg, (match) => {
24+
// Replace only the last dot with underscore (preserve the prefix character)
25+
const lastDotIdx = match.lastIndexOf('.');
26+
if (lastDotIdx > 0) {
27+
return match.substring(0, lastDotIdx) + '_';
28+
}
29+
return match;
2530
});
2631
return s;
2732
}

test/enforcer.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,26 @@ test('test ABAC single eval() with r. in unexpected places', async () => {
627627
await testEnforce(e, { id: 3 }, ({ owner: { id: 2 } } as unknown) as string, 'read', false);
628628
});
629629

630+
test('test escapeAssertion with string literals (issue)', async () => {
631+
// Test case from GitHub issue: escapeAssertion should not replace r./p. inside string literals
632+
const MY_RESOURCE_NAME = 'r.my_resource';
633+
const model = newModel();
634+
model.addDef('r', 'r', 'act, obj');
635+
model.addDef('p', 'p', 'act, obj, rule');
636+
model.addDef('e', 'e', 'some(where (p.eft == allow))');
637+
model.addDef('m', 'm', 'r.act == p.act && r.obj == p.obj && eval(p.rule)');
638+
639+
const enforcer = await newEnforcer(model);
640+
enforcer.addPolicy('alice', MY_RESOURCE_NAME, `p.obj == "${MY_RESOURCE_NAME}"`);
641+
642+
// Should work because string literals are not escaped
643+
await expect(enforcer.enforce('alice', MY_RESOURCE_NAME)).resolves.toBe(true);
644+
645+
// Test with single quotes as well
646+
enforcer.addPolicy('bob', 'p.resource', `p.obj == 'p.resource'`);
647+
await expect(enforcer.enforce('bob', 'p.resource')).resolves.toBe(true);
648+
});
649+
630650
test('TestEnforceSync', async () => {
631651
const m = newModel();
632652
m.addDef('r', 'r', 'sub, obj, act');

test/util.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,27 @@ test('bracketCompatible', () => {
228228
)
229229
).toEqual("g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act || r.obj in ['data2', 'data3'] || r.obj in ['data4', 'data5']");
230230
});
231+
232+
test('test escapeAssertion', () => {
233+
expect(util.escapeAssertion('r_sub == r_obj.value')).toEqual('r_sub == r_obj.value');
234+
expect(util.escapeAssertion('p_sub == r_sub.value')).toEqual('p_sub == r_sub.value');
235+
expect(util.escapeAssertion('r.attr.value == p.attr')).toEqual('r_attr.value == p_attr');
236+
expect(util.escapeAssertion('r.attr.value == p.attr')).toEqual('r_attr.value == p_attr');
237+
expect(util.escapeAssertion('r.attp.value || p.attr')).toEqual('r_attp.value || p_attr');
238+
expect(util.escapeAssertion('r2.attr.value == p2.attr')).toEqual('r2_attr.value == p2_attr');
239+
expect(util.escapeAssertion('r2.attp.value || p2.attr')).toEqual('r2_attp.value || p2_attr');
240+
expect(util.escapeAssertion('r.attp.value &&p.attr')).toEqual('r_attp.value &&p_attr');
241+
expect(util.escapeAssertion('r.attp.value >p.attr')).toEqual('r_attp.value >p_attr');
242+
expect(util.escapeAssertion('r.attp.value <p.attr')).toEqual('r_attp.value <p_attr');
243+
expect(util.escapeAssertion('r.attp.value +p.attr')).toEqual('r_attp.value +p_attr');
244+
expect(util.escapeAssertion('r.attp.value -p.attr')).toEqual('r_attp.value -p_attr');
245+
expect(util.escapeAssertion('r.attp.value *p.attr')).toEqual('r_attp.value *p_attr');
246+
expect(util.escapeAssertion('r.attp.value /p.attr')).toEqual('r_attp.value /p_attr');
247+
expect(util.escapeAssertion('!r.attp.value /p.attr')).toEqual('!r_attp.value /p_attr');
248+
expect(util.escapeAssertion('g(r.sub, p.sub) == p.attr')).toEqual('g(r_sub, p_sub) == p_attr');
249+
expect(util.escapeAssertion('g(r.sub,p.sub) == p.attr')).toEqual('g(r_sub,p_sub) == p_attr');
250+
expect(util.escapeAssertion('(r.attp.value || p.attr)p.u')).toEqual('(r_attp.value || p_attr)p_u');
251+
// Test that patterns inside strings are not escaped
252+
expect(util.escapeAssertion('r.sub == "a.p.p.l.e"')).toEqual('r_sub == "a.p.p.l.e"');
253+
expect(util.escapeAssertion('r.sub == "test.p.value"')).toEqual('r_sub == "test.p.value"');
254+
});

0 commit comments

Comments
 (0)