Fail closed when the gate cannot confirm an approval

src/core.ts

@@ -891,17 +891,71 @@ export function createAgentScoreCore(options: AgentScoreCoreOptions): AgentScore
 
     const decision = data.decision as string | null | undefined;
     const decisionReasons = (data.decision_reasons as string[]) ?? [];
-    const allow = decision === 'allow' || decision == null;
 
-    cache.set(cacheKey, { allow, decision: decision ?? undefined, reasons: decisionReasons, raw: data });
+    // FAIL CLOSED on a response we cannot read as an approval.
+    //
+    // This previously read `decision === 'allow' || decision == null`, so a
+    // response carrying no decision at all was treated as an approval. Combined
+    // with the API silently ignoring an unrecognised policy key, a merchant
+    // could send a misspelled rule and have both layers agree to let the request
+    // through. Only the literal string 'allow' is an approval now; null,
+    // undefined, and any value we add later are not.
+    //
+    // The two failures below are deliberately NOT handled the same way, because
+    // they are not the same kind of failure.
+    const decisionIsAllow = decision === 'allow';
+    const decisionIsUnreadable = decision == null;
 
-    // Compose the per-request signer verdicts and ride them on the RETURN VALUE so the adapter can
-    // stash them on its per-request state (NOT a shared slot — see buildSignerVerdict). guard:
-    // wallet-only identity (operator-token / AIT win and signer-match is deliberately not enforced).
+    // Computed here rather than after the allow decision: the fail-closed
+    // branches below return early and still owe the adapter these verdicts.
     const signerVerdict = buildSignerVerdict(identity, data);
-    // Rides the same assess response, so reading it later costs nothing extra.
     const operatorHandle = readOperatorHandle(data);
 
+    // A policy we SENT must come back evaluated and passing. `policy_result` is
+    // null for every ungated call, which is correct and must stay an allow, so
+    // this is conditioned on having sent a policy rather than on the field being
+    // null. Denying on a null policy_result unconditionally would deny every
+    // ungated merchant request in production.
+    const policySent = Object.keys(policy).length > 0;
+    const policyResult = data.policy_result as { all_passed?: boolean } | null | undefined;
+    const policySatisfied = !policySent || policyResult?.all_passed === true;
+
+    if (decisionIsUnreadable) {
+      // No decision field. Indistinguishable from a truncated or proxied
+      // response, so this is an INFRASTRUCTURE failure and honours the
+      // merchant's explicit `failOpen` choice, exactly as an unreachable API
+      // does above. What changed is that it is no longer a silent allow for
+      // merchants who did NOT opt into that.
+      console.warn('[gate] /v1/assess returned no decision; treating as api_error');
+      if (failOpen) return { kind: 'allow', degraded: true, infraReason: 'api_error' };
+      return { kind: 'deny', reason: { code: 'api_error' } };
+    }
+
+    if (decisionIsAllow && !policySatisfied) {
+      // An allow carrying no passing policy_result, for a request that DID send
+      // a policy. Not an availability problem: the gate was asked to enforce
+      // something and the answer does not show it was enforced. Denies
+      // regardless of `failOpen`, because failing open here would reinstate the
+      // exact hole this closes.
+      console.warn('[gate] /v1/assess allowed a request whose policy was not evaluated; denying');
+      return {
+        kind: 'deny',
+        reason: {
+          code: 'api_error',
+          decision: decision ?? undefined,
+          reasons: decisionReasons,
+          data: data as unknown as AssessResult,
+        },
+        ...(signerVerdict !== undefined && { signerVerdict }),
+        ...(operatorHandle !== undefined && { operatorHandle }),
+      };
+    }
+
+    const allow = decisionIsAllow && policySatisfied;
+
+    cache.set(cacheKey, { allow, decision: decision ?? undefined, reasons: decisionReasons, raw: data });
+
+
     if (allow) {
       // SDK populates `quota` on the assess response from X-Quota-* headers when the
       // API emits them. Surface up to the adapter so merchants can monitor approach-to-cap.

tests/express.test.ts

@@ -377,28 +377,163 @@ describe('agentscoreGate middleware — cache', () => {
 });
 
 
-describe('agentscoreGate middleware — decision null/undefined treated as allow', () => {
+describe('agentscoreGate middleware — a response with no decision fails closed', () => {
   afterEach(() => {
     vi.restoreAllMocks();
   });
 
-  it('allows when decision is null', async () => {
+  // These two previously asserted the OPPOSITE, under the name "decision
+  // null/undefined treated as allow": a response carrying no decision was let
+  // through. A penetration test found that combined with the API silently
+  // ignoring an unrecognised policy key, so a misspelled rule could pass both
+  // layers. A missing decision is now indistinguishable-from-broken and is
+  // treated as an infrastructure failure, which denies unless the merchant has
+  // explicitly opted into failOpen.
+  it('denies when decision is null', async () => {
     mockFetchOk({ ...ALLOW_RESPONSE, decision: null });
     const mw = agentscoreGate({ apiKey: API_KEY });
     const req = makeReq(WALLET);
-    const { res } = makeRes();
+    const { res, status, json } = makeRes();
     const next = makeNext();
 
     await mw(req, res, next);
 
-    expect(next).toHaveBeenCalledOnce();
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(503);
+    // api_error at 503, not wallet_not_trusted at 403: the response was
+    // unreadable, which is our failure and retryable, not a compliance verdict
+    // against the buyer. The distinction matters to an agent choosing whether to
+    // retry or to go and get verified.
+    expect(json).toHaveBeenCalledWith(expect.objectContaining({
+      error: expect.objectContaining({ code: 'api_error' }),
+    }));
   });
 
-  it('allows when decision field is missing (undefined)', async () => {
+  it('denies when the decision field is missing (undefined)', async () => {
     const { decision: _, ...noDecision } = ALLOW_RESPONSE;
     mockFetchOk(noDecision);
     const mw = agentscoreGate({ apiKey: API_KEY });
     const req = makeReq(WALLET);
+    const { res, status } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(503);
+  });
+
+  // The merchant's explicit availability choice still applies here, exactly as
+  // it does for an unreachable API: a missing decision is an unreadable
+  // response, not a compliance verdict.
+  it('honours failOpen when the merchant opted into it', async () => {
+    mockFetchOk({ ...ALLOW_RESPONSE, decision: null });
+    const mw = agentscoreGate({ apiKey: API_KEY, failOpen: true });
+    const req = makeReq(WALLET);
+    const { res } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).toHaveBeenCalledOnce();
+  });
+
+  // An unrecognised decision value must not be an allow either, so adding a new
+  // decision on the API side cannot silently open the gate on old SDKs. This one
+  // denies at 403 rather than 503: the API returned a READABLE decision that is
+  // simply not an approval, which is a verdict rather than an infrastructure
+  // failure, so it is not retryable and must not honour failOpen.
+  it('denies an unrecognised decision value', async () => {
+    mockFetchOk({ ...ALLOW_RESPONSE, decision: 'review' });
+    const mw = agentscoreGate({ apiKey: API_KEY });
+    const req = makeReq(WALLET);
+    const { res, status } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(403);
+  });
+});
+
+describe('agentscoreGate middleware — a sent policy must come back evaluated', () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  // The other half of the same finding. The API used to ignore an unrecognised
+  // policy key and answer allow with policy_result null, so the gate approved a
+  // request whose policy never ran. The API rejects that now, but the gate must
+  // not depend on the API being correct: if we ASKED for enforcement and the
+  // answer does not show it happened, that is a deny.
+  it('denies an allow whose policy_result is null when a policy was sent', async () => {
+    mockFetchOk({ ...ALLOW_RESPONSE, decision: 'allow', policy_result: null });
+    const mw = agentscoreGate({ apiKey: API_KEY, requireKyc: true });
+    const req = makeReq(WALLET);
+    const { res, status } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(503);
+  });
+
+  it('denies an allow whose policy_result did not pass', async () => {
+    mockFetchOk({
+      ...ALLOW_RESPONSE,
+      decision: 'allow',
+      policy_result: { all_passed: false, checks: [] },
+    });
+    const mw = agentscoreGate({ apiKey: API_KEY, requireKyc: true });
+    const req = makeReq(WALLET);
+    const { res, status } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(503);
+  });
+
+  // failOpen is an AVAILABILITY setting. It must not reinstate the hole: we
+  // asked for enforcement and did not get it, which is not an outage.
+  it('denies even under failOpen, because this is not an availability failure', async () => {
+    mockFetchOk({ ...ALLOW_RESPONSE, decision: 'allow', policy_result: null });
+    const mw = agentscoreGate({ apiKey: API_KEY, failOpen: true, requireKyc: true });
+    const req = makeReq(WALLET);
+    const { res } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  // The guard is conditioned on having SENT a policy. A null policy_result is
+  // the correct, normal response for every ungated call, and denying on it
+  // unconditionally would deny every ungated merchant request in production.
+  it('still allows an ungated request whose policy_result is null', async () => {
+    mockFetchOk({ ...ALLOW_RESPONSE, decision: 'allow', policy_result: null });
+    const mw = agentscoreGate({ apiKey: API_KEY });
+    const req = makeReq(WALLET);
+    const { res } = makeRes();
+    const next = makeNext();
+
+    await mw(req, res, next);
+
+    expect(next).toHaveBeenCalledOnce();
+  });
+
+  it('allows when a sent policy came back passing', async () => {
+    mockFetchOk({
+      ...ALLOW_RESPONSE,
+      decision: 'allow',
+      policy_result: { all_passed: true, checks: [{ passed: true, rule: 'require_kyc' }] },
+    });
+    const mw = agentscoreGate({ apiKey: API_KEY, requireKyc: true });
+    const req = makeReq(WALLET);
     const { res } = makeRes();
     const next = makeNext();