From d3ed0df6919b1218f5c000e6e3a3aee09035874c Mon Sep 17 00:00:00 2001 From: Michael Heller <21163552+mdheller@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:12:57 -0400 Subject: [PATCH] Require a policy decision to grant, and give callers a gate that fails closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps, both of the same shape: the rule was stated where nothing enforced it. enable() accepted policyDecisionRef = null. "enabled" is the only state that authorises use — every other state describes an observation — so recording it without naming the decision behind it produces a ledger entry asserting an adjudication that may never have happened, and one that cannot afterwards be told apart from a properly adjudicated grant. It is now required. deny() deliberately stays permissive. A refusal that cannot cite its policy is still a refusal and still fails closed; a grant that cannot cite its policy is an unattributed authorisation. The two mistakes do not cost the same, so they are not gated the same, and the asymmetry has its own test so a later reader does not "fix" it into symmetry. isEnabled() carries the instruction "feature use must be gated on this before proceeding" in a docstring, and returns a boolean. A caller who never asks proceeds exactly as if permission had been granted — the requirement fails OPEN, enforced by whoever happened to read the comment. assertEnabled() throws instead, naming the state actually recorded and the policy decision behind it, so a refusal is diagnosable rather than opaque. Writing the negative controls found a bug in the change itself: getState() reports an unknown capability as null, not undefined, so the "not in the ledger" branch was unreachable and an undeclared capability would have produced a confusing 'is "null", not "enabled"' message. The test that exercises every non-enabled state is the one that matters — a capability part-way through its lifecycle must refuse, not pass through. Seven existing tests called enable() with no policy reference. They were exercising reconcile, conflict and timestamp behaviour rather than the permissiveness of enable, so each now names the decision that authorised it. --- packages/capability-ledger/src/index.js | 59 +++++++++++- .../capability-ledger/tests/ledger.test.js | 92 +++++++++++++++++-- 2 files changed, 142 insertions(+), 9 deletions(-) diff --git a/packages/capability-ledger/src/index.js b/packages/capability-ledger/src/index.js index ff737b5..662eba9 100644 --- a/packages/capability-ledger/src/index.js +++ b/packages/capability-ledger/src/index.js @@ -113,17 +113,37 @@ export class CapabilityLedger { /** * Enable a capability. + * + * `policyDecisionRef` is REQUIRED. "enabled" is the only state that authorises use, + * so recording it without naming the decision that authorised it produces a ledger + * entry asserting an adjudication that may never have occurred — and, being + * indistinguishable from a properly adjudicated one, it cannot be audited apart from + * it afterwards. Every other state describes an observation; only this one grants. + * * @param {string} capabilityId * @param {CapabilityOwner} owner - * @param {string|null} policyDecisionRef + * @param {string} policyDecisionRef reference to the decision that authorised this * @param {string[]} [evidenceRefs] */ - enable(capabilityId, owner, policyDecisionRef = null, evidenceRefs = []) { + enable(capabilityId, owner, policyDecisionRef, evidenceRefs = []) { + if (typeof policyDecisionRef !== 'string' || policyDecisionRef.trim() === '') { + throw new Error( + `capability "${capabilityId}" cannot be enabled without a policyDecisionRef: ` + + 'enabling is the only state that authorises use, and an unattributed grant is ' + + 'indistinguishable from an adjudicated one', + ); + } return this._emit(capabilityId, 'enabled', owner, { policyDecisionRef, evidenceRefs }); } /** * Deny a capability via policy. + * + * `policyDecisionRef` remains optional here, deliberately and asymmetrically: a + * refusal that cannot cite its policy is still a refusal and still fails closed, + * whereas a grant that cannot cite its policy is an unattributed authorisation. + * The two mistakes do not cost the same, so they are not gated the same. + * * @param {string} capabilityId * @param {CapabilityOwner} owner * @param {string|null} policyDecisionRef @@ -246,10 +266,45 @@ export class CapabilityLedger { /** * Returns true only when the ledger reports the capability as "enabled". * Feature use must be gated on this before proceeding. + * + * Note this returns a value a caller may ignore, and every caller that forgets it + * fails OPEN. Prefer {@link assertEnabled} where the call site can throw. + * * @param {string} capabilityId * @returns {boolean} */ isEnabled(capabilityId) { return this.getState(capabilityId) === 'enabled'; } + + /** + * Fail-closed gate: throw unless the capability is enabled. + * + * `isEnabled` states the requirement in a docstring — "feature use must be gated on + * this" — and returns a boolean, so a caller that never asks proceeds exactly as if + * permission had been granted. A requirement whose only enforcement is a sentence in + * a comment is enforced by whoever happened to read the comment. + * + * The thrown error names the state actually recorded, and the policy decision behind + * it when there is one, so a refusal is diagnosable rather than merely opaque. + * + * @param {string} capabilityId + * @returns {import('./schema.js').CapabilityReceipt} the receipt that authorised use + * @throws {Error} when the capability is not enabled + */ + assertEnabled(capabilityId) { + const state = this.getState(capabilityId); + if (state === 'enabled') return this.getReceipt(capabilityId); + + const receipt = this.getReceipt(capabilityId); + const because = receipt?.policyDecisionRef + ? ` (policy decision: ${receipt.policyDecisionRef})` + : ''; + throw new Error( + // getState() reports an unknown capability as null, not undefined. + state == null + ? `capability "${capabilityId}" is not in the ledger; refusing use of an undeclared capability` + : `capability "${capabilityId}" is "${state}", not "enabled"${because}; refusing use`, + ); + } } diff --git a/packages/capability-ledger/tests/ledger.test.js b/packages/capability-ledger/tests/ledger.test.js index 944e469..9eb883e 100644 --- a/packages/capability-ledger/tests/ledger.test.js +++ b/packages/capability-ledger/tests/ledger.test.js @@ -183,7 +183,7 @@ describe('failed reconciliation', () => { test('reconcile reports failed capability as pending', () => { const ledger = new CapabilityLedger(); - ledger.enable('cap-a', 'runtime'); + ledger.enable('cap-a', 'runtime', 'policy:test-grant:cap-a'); ledger.fail('cap-b', 'runtime', []); const { enabled, pending } = ledger.reconcile(); @@ -193,8 +193,8 @@ describe('failed reconciliation', () => { test('reconcile returns all enabled capabilities', () => { const ledger = new CapabilityLedger(); - ledger.enable('cap-a', 'runtime'); - ledger.enable('cap-b', 'runtime'); + ledger.enable('cap-a', 'runtime', 'policy:test-grant:cap-a'); + ledger.enable('cap-b', 'runtime', 'policy:test-grant:cap-b'); const { enabled, pending } = ledger.reconcile(); assert.deepEqual(enabled.sort(), ['cap-a', 'cap-b']); @@ -228,7 +228,7 @@ describe('conflict warnings', () => { const ledger = new CapabilityLedger(); ledger.declare('cap-x', 'runtime'); ledger.logConflict('cap-x', 'warning 1'); - ledger.enable('cap-x', 'runtime'); + ledger.enable('cap-x', 'runtime', 'policy:test-grant:cap-x'); const receipt = ledger.getReceipt('cap-x'); assert.equal(receipt.state, 'enabled'); @@ -237,7 +237,7 @@ describe('conflict warnings', () => { test('reconcile includes conflicted capabilities', () => { const ledger = new CapabilityLedger(); - ledger.enable('cap-conflict', 'runtime'); + ledger.enable('cap-conflict', 'runtime', 'policy:test-grant:cap-conflict'); ledger.logConflict('cap-conflict', 'server disagrees'); const { conflicted } = ledger.reconcile(); @@ -250,7 +250,7 @@ describe('conflict warnings', () => { describe('getAll', () => { test('returns all tracked receipts', () => { const ledger = new CapabilityLedger(); - ledger.enable('a', 'runtime'); + ledger.enable('a', 'runtime', 'policy:test-grant:a'); ledger.deny('b', 'policy', null, []); const all = ledger.getAll(); @@ -265,7 +265,7 @@ describe('getAll', () => { describe('receipt timestamp', () => { test('timestamp is a valid ISO-8601 date string', () => { const ledger = new CapabilityLedger(); - ledger.enable('ts-test', 'runtime'); + ledger.enable('ts-test', 'runtime', 'policy:test-grant:ts-test'); const receipt = ledger.getReceipt('ts-test'); const parsed = new Date(receipt.timestamp); assert.ok(!isNaN(parsed.getTime()), 'timestamp is not a valid date'); @@ -322,3 +322,81 @@ describe('full lifecycle', () => { assert.equal(ledger.isEnabled('degrade-test'), false); }); }); + +describe('a grant must name the decision that authorised it', () => { + // "enabled" is the only state that authorises use. Recorded without a + // policyDecisionRef it asserts an adjudication that may never have happened, and + // is then indistinguishable from a properly adjudicated grant when audited. + test('enable() refuses without a policyDecisionRef', () => { + const ledger = new CapabilityLedger(); + assert.throws(() => ledger.enable('pdf-viewer', 'runtime'), /without a policyDecisionRef/); + assert.equal(ledger.getState('pdf-viewer'), null, 'a refused grant must leave no trace of enablement'); + assert.equal(ledger.isEnabled('pdf-viewer'), false); + }); + + test('enable() refuses an empty or whitespace policyDecisionRef', () => { + const ledger = new CapabilityLedger(); + assert.throws(() => ledger.enable('a', 'runtime', ''), /without a policyDecisionRef/); + assert.throws(() => ledger.enable('a', 'runtime', ' '), /without a policyDecisionRef/); + assert.throws(() => ledger.enable('a', 'runtime', null), /without a policyDecisionRef/); + assert.equal(ledger.isEnabled('a'), false); + }); + + test('enable() proceeds when the decision is named', () => { + const ledger = new CapabilityLedger(); + ledger.enable('pdf-viewer', 'runtime', 'policy:allow-pdf:v1', ['config:pdf:on']); + assert.equal(ledger.isEnabled('pdf-viewer'), true); + assert.equal(ledger.getReceipt('pdf-viewer').policyDecisionRef, 'policy:allow-pdf:v1'); + }); + + test('deny() stays permissive without a policy ref — the asymmetry is deliberate', () => { + // A refusal that cannot cite its policy still fails closed. A grant that cannot + // cite its policy is an unattributed authorisation. Different costs, different gates. + const ledger = new CapabilityLedger(); + ledger.deny('restricted', 'policy'); + assert.equal(ledger.getState('restricted'), 'blocked_by_policy'); + assert.equal(ledger.isEnabled('restricted'), false); + }); +}); + +describe('assertEnabled fails closed where isEnabled fails open', () => { + test('returns the authorising receipt when enabled', () => { + const ledger = new CapabilityLedger(); + ledger.enable('pdf-viewer', 'runtime', 'policy:allow-pdf:v1'); + const receipt = ledger.assertEnabled('pdf-viewer'); + assert.equal(receipt.state, 'enabled'); + assert.equal(receipt.policyDecisionRef, 'policy:allow-pdf:v1'); + }); + + test('throws on a denied capability, naming the state and the policy behind it', () => { + const ledger = new CapabilityLedger(); + ledger.deny('restricted', 'policy', 'policy:deny-restricted:v2'); + assert.throws(() => ledger.assertEnabled('restricted'), /blocked_by_policy/); + assert.throws(() => ledger.assertEnabled('restricted'), /policy:deny-restricted:v2/); + }); + + test('throws on an undeclared capability rather than treating absence as permission', () => { + const ledger = new CapabilityLedger(); + assert.throws(() => ledger.assertEnabled('never-declared'), /not in the ledger/); + }); + + test('throws on every non-enabled state', () => { + // The negative control that matters: a capability part-way through its lifecycle + // is not usable, and each intermediate state must refuse rather than pass through. + for (const [id, drive] of [ + ['declared', (l) => l.declare('c', 'runtime')], + ['requested', (l) => l.request('c', 'UI')], + ['negotiating', (l) => l.negotiate('c', 'runtime')], + ['available', (l) => l.setAvailable('c', 'server')], + ['degraded', (l) => l.degrade('c', 'runtime')], + ['unsupported_by_runtime', (l) => l.setUnsupportedByRuntime('c', 'runtime')], + ['missing_plugin', (l) => l.setMissingPlugin('c', 'plugin')], + ['failed', (l) => l.fail('c', 'runtime')], + ]) { + const ledger = new CapabilityLedger(); + drive(ledger); + assert.equal(ledger.isEnabled('c'), false, `${id} must not read as enabled`); + assert.throws(() => ledger.assertEnabled('c'), new RegExp(id), `${id} must refuse use`); + } + }); +});