fix(acl): hard-reject a lapsed token binding instead of prompting (#36)
Some checks failed
Docker image / build-and-push-image (push) Has been cancelled

A request that finds no live token grant exited `checkIfPubkeyAllowed`
at `undefined` regardless of *why* — whether the binding never existed
or had simply lapsed (expired / token-revoked). `undefined` routes the
caller into the admin-prompt path, which for an unattended client (an
ATM spire) means the request hangs until a BunkerTimeoutError.

The Sintra smoke proved the divergence directly: a KeyUser-level revoke
exits at step 2 with `false` and the spire sees a clean BunkerRejected
("Pairing Required"), but a TTL expiry fell through to `undefined` and
the spire saw a BunkerTimeout ("Signer Unreachable") — same operator
intent ("this pairing is over"), two different, one-broken outcomes.

Classify the no-live-grant case before returning: if a token bound to
this KeyUser *would* have granted the request (its policy carries a
matching rule; for `connect`, any bound token) but is now expired or
token-revoked, return `false` so the client re-pairs immediately. Only
a genuinely never-granted (method/kind) request stays `undefined` so an
admin can still approve new permission out-of-band.

Usage-cap exhaustion is left at `undefined` deliberately: a windowed
cap is a temporary rate-limit that refills as the window rolls, not a
permanent lapse, so it must not be reclassed as the re-pair signal. A
dedicated rate-limit reply is a separate follow-up.

Tests: the #24 expired-token and token-revoke guards now assert `false`;
added connect-lapse, and two distinction cases proving a never-granted
method (live token, or a method the lapsed token never covered) stays
`undefined`.
This commit is contained in:
Padreug 2026-06-21 12:44:36 +02:00
commit 14d48ca0f9
2 changed files with 85 additions and 27 deletions

View file

@ -149,18 +149,25 @@ test('live token + matching policy rule -> sign_event allowed', async () => {
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), true);
});
test('expired token -> sign_event denied [#24 regression guard]', async () => {
// A token bound to the KeyUser that has lapsed (expiry or token-revoke) means
// the pairing WAS granted and is now spent. It must hard-reject with `false` so
// an unattended client re-pairs immediately, NOT `undefined` (which routes to
// the admin-prompt path and hangs an ATM until a BunkerTimeoutError). The smoke
// on the Sintra proved the divergence: revoke -> clean reject, expiry -> hang.
// See aiolabs/nsecbunkerd#36 (and #24, which made the expired token stop
// granting in the first place).
test('expired token -> sign_event hard-rejected (false) [#24 + #36]', async () => {
const pid = await seedPolicy('sign_event', '1');
const ku = await seedKeyUser();
await seedToken(ku, pid, { expiresAt: past() });
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), undefined);
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), false);
});
test('revoked token -> sign_event denied', async () => {
test('token-revoked -> sign_event hard-rejected (false) [#36]', async () => {
const pid = await seedPolicy('sign_event', '1');
const ku = await seedKeyUser();
await seedToken(ku, pid, { revokedAt: past() });
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), undefined);
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), false);
});
test('live token -> connect allowed (pairing)', async () => {
@ -170,11 +177,31 @@ test('live token -> connect allowed (pairing)', async () => {
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), true);
});
test('expired token -> connect denied', async () => {
test('expired token -> connect hard-rejected (false) [#36]', async () => {
const pid = await seedPolicy('sign_event', '1');
const ku = await seedKeyUser();
await seedToken(ku, pid, { expiresAt: past() });
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), undefined);
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), false);
});
// The reject above is reserved for a binding that LAPSED. A request whose method
// was never in the (still-live) token's policy is genuinely new permission and
// must stay `undefined` so an admin could approve it out-of-band — it must NOT
// be swept up by the #36 re-pair signal.
test('live token, method outside its policy -> undefined (never granted, not lapsed) [#36]', async () => {
const pid = await seedPolicy('sign_event', '1');
const ku = await seedKeyUser();
await seedToken(ku, pid, {});
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'nip44_encrypt'), undefined);
});
// And a lapsed token only rejects the method IT covered: a different method has
// no lapsed grant of its own, so it stays a never-granted `undefined`.
test('expired token, request a method it never covered -> undefined [#36]', async () => {
const pid = await seedPolicy('sign_event', '1');
const ku = await seedKeyUser();
await seedToken(ku, pid, { expiresAt: past() });
assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'nip44_encrypt'), undefined);
});
test('KeyUser.revokedAt denies (false) and beats a live token', async () => {