diff --git a/.changeset/strict-policies-deny.md b/.changeset/strict-policies-deny.md new file mode 100644 index 000000000000..3af284550521 --- /dev/null +++ b/.changeset/strict-policies-deny.md @@ -0,0 +1,5 @@ +--- +'@ai-sdk/policy-opa': patch +--- + +fix(policy-opa): deny tool execution when OPA returns an unrecognized decision diff --git a/content/docs/03-agents/06-policy-tool-approvals.mdx b/content/docs/03-agents/06-policy-tool-approvals.mdx index 3a3dc88cdbae..63a245cd55e7 100644 --- a/content/docs/03-agents/06-policy-tool-approvals.mdx +++ b/content/docs/03-agents/06-policy-tool-approvals.mdx @@ -156,9 +156,10 @@ Run with `opa test policy.rego policy_test.rego`. ### Errors fail closed - If the backend errors (server unreachable, WASM fault, misbuilt bundle), `opaPolicy` returns `denied` with the error message as the reason. +- If the backend returns a present but unrecognized decision, such as an unknown `decision` value or a non-boolean legacy `allow` value, `opaPolicy` returns `denied`. - The error never rejects out of the callback and never aborts the run. - A backend outage blocks the affected call rather than silently allowing it. -- This is distinct from a rule that returns no match, which normalizes to `not-applicable` (allow). Use a `default ... deny` rule if you want unmatched calls denied too. +- This is distinct from a rule that returns no match: `null` or `undefined` normalizes to `not-applicable` (allow). Use a `default ... deny` rule if you want unmatched calls denied too. ## Loading the policy diff --git a/packages/policy-opa/README.md b/packages/policy-opa/README.md index 28e16e417a0e..de05abb8f708 100644 --- a/packages/policy-opa/README.md +++ b/packages/policy-opa/README.md @@ -107,7 +107,9 @@ The adapter also accepts the legacy boolean shape (`{ "allow": true | false, "re If the backend itself errors (OPA server unreachable, WASM fault, a misbuilt bundle that yields no result), `opaPolicy` returns `{ type: 'denied' }` with the underlying message as the reason. The error never rejects out of the `toolApproval` callback and never aborts the run. A backend outage blocks the affected tool call rather than silently letting it through. This matches `opaCapabilityMiddleware`, which also fails closed. -Note this is distinct from a Rego rule that returns no matching decision: that normalizes to `not-applicable` ("no opinion"), which the SDK treats as allow. Use `default decision := { "decision": "deny" }` in your policy if you want unmatched calls to be denied too. +Present but unrecognized policy results, such as an unknown `decision` value or a non-boolean legacy `allow` value, also fail closed with a denied result. + +This is distinct from a Rego rule that returns no matching decision: `null` or `undefined` normalizes to `not-applicable` ("no opinion"), which the SDK treats as allow. Use `default decision := { "decision": "deny" }` in your policy if you want unmatched calls to be denied too. ### What the adapter passes as `input` diff --git a/packages/policy-opa/src/opa/normalize-opa-decision.test.ts b/packages/policy-opa/src/opa/normalize-opa-decision.test.ts index 42cf83f90ca0..85e9fcda4ebd 100644 --- a/packages/policy-opa/src/opa/normalize-opa-decision.test.ts +++ b/packages/policy-opa/src/opa/normalize-opa-decision.test.ts @@ -80,15 +80,17 @@ describe('normalizeOpaDecision', () => { }); }); - it('treats unrecognized shape as not-applicable', () => { - expect(normalizeOpaDecision({ result: 'maybe' })).toEqual({ - type: 'not-applicable', + it.each([ + ['an unknown decision value', { decision: 'blocked' }], + ['a non-boolean legacy value', { allow: 'false' }], + ['an unknown key', { verdict: 'deny' }], + ['an unexpected nested shape', { result: { decision: 'deny' } }], + ['a primitive', 'yes'], + ])('denies %s', (_description, result) => { + expect(normalizeOpaDecision(result)).toEqual({ + type: 'denied', + reason: 'unrecognized OPA policy decision', }); }); - - it('treats primitives as not-applicable', () => { - expect(normalizeOpaDecision('yes')).toEqual({ type: 'not-applicable' }); - expect(normalizeOpaDecision(42)).toEqual({ type: 'not-applicable' }); - }); }); }); diff --git a/packages/policy-opa/src/opa/normalize-opa-decision.ts b/packages/policy-opa/src/opa/normalize-opa-decision.ts index ba355f5f1a15..71eeb839e833 100644 --- a/packages/policy-opa/src/opa/normalize-opa-decision.ts +++ b/packages/policy-opa/src/opa/normalize-opa-decision.ts @@ -12,9 +12,10 @@ import type { PolicyDecision } from '../policy-decision'; * - **Legacy (boolean):** `{ "allow": boolean, "reason"?: string }`. `true` * maps to `approved`, `false` to `denied`. * - * Unknown shapes and `undefined` are treated as `not-applicable` so that a - * Rego rule that does not match any branch defaults to "no opinion" rather - * than blocking. + * `null` and `undefined` are treated as `not-applicable` so that a Rego rule + * that does not match any branch defaults to "no opinion" rather than + * blocking. Any other unrecognized result is denied so malformed policy + * output cannot silently bypass the approval gate. */ export function normalizeOpaDecision(result: unknown): PolicyDecision { if (result == null) { @@ -22,7 +23,7 @@ export function normalizeOpaDecision(result: unknown): PolicyDecision { } if (typeof result !== 'object') { - return { type: 'not-applicable' }; + return unrecognizedDecision(); } const record = result as Record; @@ -45,7 +46,14 @@ export function normalizeOpaDecision(result: unknown): PolicyDecision { return withReason(record.allow ? 'approved' : 'denied', reason); } - return { type: 'not-applicable' }; + return unrecognizedDecision(); +} + +function unrecognizedDecision(): PolicyDecision { + return { + type: 'denied', + reason: 'unrecognized OPA policy decision', + }; } function withReason( diff --git a/packages/policy-opa/src/opa/opa-policy.integration.test.ts b/packages/policy-opa/src/opa/opa-policy.integration.test.ts index 9c8d4b2ff716..aee4104c3899 100644 --- a/packages/policy-opa/src/opa/opa-policy.integration.test.ts +++ b/packages/policy-opa/src/opa/opa-policy.integration.test.ts @@ -142,6 +142,45 @@ describe('opaPolicy end-to-end with generateText', () => { }); }); + it('skips execution when the policy returns an unrecognized decision', async () => { + const execute = vi.fn(async () => 'ok'); + + const result = await generateText({ + model: modelEmittingOneToolCallThenText(), + prompt: 'do something', + stopWhen: isStepCount(3), + tools: { + git: tool({ + inputSchema: jsonSchema<{ args: string[] }>({ + type: 'object', + properties: { args: { type: 'array', items: { type: 'string' } } }, + required: ['args'], + }), + execute, + }), + }, + toolApproval: opaPolicy({ + client: stubClient({ decision: 'blocked' }), + path: 'agent/call/decision', + }), + }); + + expect(execute).not.toHaveBeenCalled(); + const toolMessage = result.responseMessages.find(m => m.role === 'tool') as + | { + content: Array<{ + type: string; + output?: { type: string; reason?: string }; + }>; + } + | undefined; + const toolResult = toolMessage?.content.find(c => c.type === 'tool-result'); + expect(toolResult?.output).toEqual({ + type: 'execution-denied', + reason: 'unrecognized OPA policy decision', + }); + }); + it('routes a dispatcher call through the same Rego rule via toInput', async () => { // Demonstrates the transitive-enforcement pattern documented in the // README: bash `'git push'` is rewritten to (kind: 'git', args: ['push'])