+ "details": "### Summary\nBypass policies incorrectly authorize requests when their condition evaluates to true but their authorization checks fail and no other policies apply.\n\n### Impact\nResources with bypass policies can be accessed without proper authorization when:\n- Bypass condition evaluates to true\n- Bypass authorization checks fail\n- Other policies exist but their conditions don't match\n\n### Details\nBug introduced in PR #2365 (commit 79749c26).\n\nAffected line: [lib/ash/policy/policy.ex:69](https://github.com/ash-project/ash/blob/b2e4d625/lib/ash/policy/policy.ex#L69)\n```elixir\n{%{bypass?: true}, cond_expr, complete_expr}, {one_condition_matches, all_policies_match} ->\n {\n b(cond_expr or one_condition_matches), # <- Bug: uses condition only\n b(complete_expr or all_policies_match)\n }\n```\n\nThe final authorization decision is: `one_condition_matches AND all_policies_match`\n\nWhen a bypass condition is true but bypass policies fail, and subsequent policies have non-matching conditions:\n\n1. **one_condition_matches** = `cond_expr` (bypass condition) = **true** (bug - should check if bypass actually authorizes)\n2. **all_policies_match** = `(complete_expr OR NOT cond_expr)` for each policy\n - For non-matching policies: `(false OR NOT false)` = **true** (policies don't apply)\n3. **Final**: `true AND true` = **true** (incorrectly authorized)\n\nThe bypass condition alone satisfies \"at least one policy applies\" even though the bypass fails to authorize.\n\n### Fix\nReplace `cond_expr` with `complete_expr` on line 69:\n```elixir\n{%{bypass?: true}, _cond_expr, complete_expr}, {one_condition_matches, all_policies_match} ->\n {\n b(complete_expr or one_condition_matches), # <- Fixed\n b(complete_expr or all_policies_match)\n }\n```\n\nLine 52 should also be updated for consistency (though it's only triggered when bypass is the last policy, making it coincidentally safe in practice):\n```elixir\n{%{bypass?: true}, _cond_expr, complete_expr}, {one_condition_matches, true} ->\n {\n b(complete_expr or one_condition_matches), # <- For consistency\n complete_expr\n }\n```\n\n### PoC\n```elixir\npolicies do\n bypass always() do\n authorize_if actor_attribute_equals(:is_admin, true)\n end\n\n policy action_type(:read) do\n authorize_if always()\n end\nend\n```\n\nNon-admin user can perform create actions (should be denied).\n\nTest demonstrating the bug:\n```elixir\ntest \"bypass policy bug\" do\n policies = [\n %Ash.Policy.Policy{\n bypass?: true,\n condition: [{Ash.Policy.Check.Static, result: true}], # condition = true\n policies: [\n %Ash.Policy.Check{\n type: :authorize_if,\n check: {Ash.Policy.Check.Static, result: false}, # policies = false\n check_module: Ash.Policy.Check.Static,\n check_opts: [result: false]\n }\n ]\n },\n %Ash.Policy.Policy{\n bypass?: false,\n condition: [{Ash.Policy.Check.Static, result: false}],\n policies: [\n %Ash.Policy.Check{\n type: :authorize_if,\n check: {Ash.Policy.Check.Static, result: true},\n check_module: Ash.Policy.Check.Static,\n check_opts: [result: true]\n }\n ]\n }\n ]\n\n expression = Ash.Policy.Policy.expression(policies, %{})\n \n assert expression == false\n # Expected: false (deny)\n # Actual on main: true (incorrectly authorized)\nend\n```",
0 commit comments