Skip to content

Commit 787fef5

Browse files
committed
test(conformance): the index matcher was pinned to one spelling (apcore#112)
The driver read the refused rule index with `/Rule (\d+)/`. The two axis families are raised from different code paths and need not word the index identically — this SDK's per-rule validator says `Rule 0 has invalid effect`, while apcore-rust's loader-only key axis says `ACL rule 1 in '<file>' carries 'priority' unrecognised` — and a matcher pinned to one spelling reads the other as naming NO rule. That is a silent pass on `lowest_indexed_bad_rule_wins_over_a_loader_only_axis`, the case built to catch a real bug. **This SDK's spellings do not differ**: every rejection it raises, the rule-key closure included, opens with `Rule N` — measured across all seven loader messages. So nothing was passing falsely here. The driver is hardened anyway, because its job is to detect a divergence rather than to encode this implementation's current phrasing, and a message reworded later would have degraded to a false pass rather than a failure. `ruleIndicesNamed()` now matches `\brules?\s+(\d+)\b` case-insensitively and returns every index found, and the assertion requires the whole set to be exactly the expected one — so "names no rule", "names the wrong rule" and "names two rules" all fail rather than the first two passing. Prose about "the rule" is not a hit: a digit must follow the word, or this SDK's own pattern-shape message ("makes the rule inert") would read as naming rule 0. `default_effect_is_judged_before_any_rule` asserts the index set is EMPTY rather than that a regex failed to match, so a wording the driver does not recognise cannot read as "no index present" — the same trap pointing the other way. A driver self-test pins the matcher against both spellings, the two-index case and the two prose non-hits, so this cannot come back silently. The SDK-local suite gets the same treatment where it asserts an index is absent, plus a test that both of this SDK's axis families word the index the same way — the thing that would have to change for the trap to matter here. Refs: apcore#112 Signed-off-by: tercel <tercel.yi@gmail.com>
1 parent 57b718e commit 787fef5

2 files changed

Lines changed: 81 additions & 13 deletions

File tree

tests/acl-pattern-arity_spec.test.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,12 @@ describe('§6.2.1 point 2: rule index dominates effect -> approval -> patterns',
601601
];
602602
for (const message of messages) {
603603
expect(message).toMatch(/Invalid default_effect 'Allow'/);
604-
expect(message).not.toMatch(/Rule \d+/);
604+
// No rule index in ANY spelling: its presence would mean a rule was
605+
// judged ahead of the file's own effect. Case-insensitive because the
606+
// check is about whether a rule was reached, not about this SDK's
607+
// capitalisation — the two axis families are raised from different code
608+
// paths and need not word the index identically.
609+
expect(message).not.toMatch(/\brules?\s+\d+\b/i);
605610
}
606611
});
607612

@@ -622,9 +627,27 @@ describe('§6.2.1 point 2: rule index dominates effect -> approval -> patterns',
622627
);
623628
const message = messageFrom(() => ACL.load(file));
624629
expect(message).toMatch(/Rule 0 has invalid effect 'Allow'/);
625-
expect(message).not.toMatch(/Rule 1/);
630+
expect(message).not.toMatch(/\brules?\s+1\b/i);
626631
expect(message).not.toMatch(/priority/);
627632
});
633+
634+
it('words the rule index the same way on the loader-only axis', () => {
635+
// The two axis families are raised from different code paths — the
636+
// rule-key closure (#107, loader-only) and the per-rule validator — and an
637+
// implementation whose spellings differ makes a driver reading the index
638+
// off the message miss one of them entirely. Here both say `Rule N`, and
639+
// this is the test that notices if one of them stops.
640+
const unknownKey = messageFrom(() =>
641+
ACL.load(
642+
writeRawAclFile([{ callers: ['*'], targets: ['*'], effect: 'allow', priority: 3 }], 'deny'),
643+
),
644+
);
645+
expect(unknownKey).toMatch(/^Rule 0 carries 'priority'/);
646+
const badEffect = messageFrom(() =>
647+
ACL.load(writeRawAclFile([{ callers: ['*'], targets: ['*'], effect: 'Allow' }], 'deny')),
648+
);
649+
expect(badEffect).toMatch(/^Rule 0 has invalid effect/);
650+
});
628651
});
629652

630653
// ---------------------------------------------------------------------------

tests/conformance-acl-pattern-arity.test.ts

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,14 @@ function assertRefusedAxis(caseId: string, axis: string, message: string): void
266266
expect(message, `${caseId}: refusal did not name default_effect: ${message}`).toMatch(
267267
/Invalid default_effect/,
268268
);
269-
expect(message, `${caseId}: a rule was judged first: ${message}`).not.toMatch(/Rule \d+/);
269+
// No rule index, in ANY spelling — its presence would mean a rule was
270+
// judged ahead of the file's own effect. Asserted as an empty set rather
271+
// than as a failed match, so a wording this driver does not recognise
272+
// cannot read as "no index present".
273+
expect(
274+
ruleIndicesNamed(message),
275+
`${caseId}: a rule was judged before default_effect: ${message}`,
276+
).toEqual([]);
270277
return;
271278
case 'effect':
272279
expect(message, `${caseId}: refusal did not name the effect: ${message}`).toMatch(
@@ -304,19 +311,40 @@ function assertRefusedAxis(caseId: string, axis: string, message: string): void
304311
}
305312

306313
/**
307-
* Assert that a refusal names the rule the case declares (§6.2.1 point 2).
314+
* Every rule index a refusal names, in any spelling a door might use.
315+
*
316+
* Deliberately case-insensitive and tolerant of a prefix: the axis families are
317+
* raised from different code paths and need not word the index identically —
318+
* this SDK's per-rule validator says `Rule 0 has invalid effect`, while another
319+
* implementation's loader-only key axis says `ACL rule 1 in '<file>' carries
320+
* 'priority' unrecognised`. A matcher pinned to one spelling reads the other as
321+
* naming NO rule, which is a silent pass on exactly the case built to catch a
322+
* real bug (measured in apcore-rust on
323+
* `lowest_indexed_bad_rule_wins_over_a_loader_only_axis`).
308324
*
309-
* Every rejection message in this SDK opens with `Rule <index>`, so the index
310-
* is read back off the message rather than inferred. The index chooses the
311-
* rule and the axis order then chooses the fault inside it: a driver asserting
312-
* only the axis passes a rule set refused for the wrong rule on the right axis,
313-
* which is exactly the shape of the loader/constructor divergence these cases
314-
* were written for.
325+
* A digit must follow the word, so the prose in this SDK's own pattern-shape
326+
* message — "makes the rule inert", "an inert deny rule under…" — is not a hit.
327+
*/
328+
function ruleIndicesNamed(message: string): number[] {
329+
return [...message.matchAll(/\brules?\s+(\d+)\b/gi)].map((m) => Number(m[1]));
330+
}
331+
332+
/**
333+
* Assert that a refusal names the rule the case declares, and only that rule
334+
* (§6.2.1 point 2).
335+
*
336+
* The index chooses the rule and the axis order then chooses the fault inside
337+
* it, so a driver asserting only the axis passes a rule set refused for the
338+
* WRONG rule on the right axis — the shape of the loader/constructor divergence
339+
* these cases were written for. Asserting the whole set rather than the first
340+
* match also catches a message naming no rule at all and one naming two.
315341
*/
316342
function assertRefusedRuleIndex(caseId: string, index: number, message: string): void {
317-
const named = /Rule (\d+)/.exec(message);
318-
expect(named, `${caseId}: refusal named no rule index: ${message}`).not.toBeNull();
319-
expect(Number(named?.[1]), `${caseId}: refusal named the wrong rule: ${message}`).toBe(index);
343+
const named = new Set(ruleIndicesNamed(message));
344+
expect(
345+
[...named],
346+
`${caseId}: refusal should name rule ${index} and no other: ${message}`,
347+
).toEqual([index]);
320348
}
321349

322350
/** The mutations a backstop case declares, as a list whatever its shape. */
@@ -339,6 +367,23 @@ describeIfPresent("Conformance: a pattern array's arity is closed (§6.2.1, spec
339367
}
340368
});
341369

370+
it('reads a rule index in every spelling a door might use', () => {
371+
// The driver's own guard. The axis families are raised from different code
372+
// paths and need not word the index the same way; a matcher pinned to one
373+
// spelling reads the others as naming NO rule, which is a silent pass on
374+
// the very cases that pin the ordering. This is the shape that bit
375+
// apcore-rust, whose loader-only key axis says `ACL rule 1 in '<file>'`.
376+
expect(ruleIndicesNamed("Rule 0 has invalid effect 'Allow'")).toEqual([0]);
377+
expect(ruleIndicesNamed("ACL rule 1 in '/tmp/acl.yaml' carries 'priority'")).toEqual([1]);
378+
expect(ruleIndicesNamed('rules 2 and Rule 3')).toEqual([2, 3]);
379+
expect(ruleIndicesNamed("Invalid default_effect 'Allow', must be 'allow' or 'deny'")).toEqual(
380+
[],
381+
);
382+
// Prose about "the rule" is not an index, or every pattern-shape message in
383+
// this SDK — "makes the rule inert" — would read as naming one.
384+
expect(ruleIndicesNamed('a shape that can never match makes the rule inert')).toEqual([]);
385+
});
386+
342387
it('exercises every case the fixture declares', () => {
343388
// "N skipped" and "N satisfied by construction" are different claims. This
344389
// SDK reaches every door and the installed-rule mutation route, so the

0 commit comments

Comments
 (0)