Skip to content

Fix compile cycle with domain code_interface + append default policies#1

Closed
barnabasJ wants to merge 2 commits intoteam-alembic:mainfrom
barnabasJ:fix/domain-merge-cycle
Closed

Fix compile cycle with domain code_interface + append default policies#1
barnabasJ wants to merge 2 commits intoteam-alembic:mainfrom
barnabasJ:fix/domain-merge-cycle

Conversation

@barnabasJ
Copy link
Copy Markdown

Summary

Two independent fixes surfaced while adopting ash_grant in a real codebase
(ARCC Center — Ash 3.x, ~57 resources, domain-level code_interface blocks).

1. cfce2b4 — append default policies after user-defined ones

AshGrant.Transformers.AddDefaultPolicies was prepending its generated
policies via Ash.Resource.Builder.add_policy/3 (which defaults to
type: :prepend). Ash bypass policies only short-circuit policies that
come after them, so user-defined bypass action_type(:create) blocks
(e.g. "allow public registration for nil actor") never fired — ash_grant's
permission check ran first and denied.

Change: flip the builder call to type: :append so user-defined policies
(including bypasses) are evaluated first and can properly short-circuit
ash_grant's generated checks. 6-line diff.

2. e56b814 — break domain↔resource compile cycle with domain-level code_interface

Combining AshGrant on a resource with AshGrant.Domain + a
code_interface do define … end block on the domain deadlocked at compile:

  • The resource's MergeDomainConfig transformer forced the domain to compile
    (to read domain-level ash_grant DSL).
  • The domain's code_interface transformer was waiting for the resource to
    finish compiling.

Fix:

  • Remove MergeDomainConfig transformer. Move resolver/scope merging
    from compile-time into runtime AshGrant.Info calls:
    resolver/1 now falls back to the domain; scopes/1 merges resource +
    domain scopes with resource precedence.
  • Convert compile-time validators to Spark verifiers.
    ValidateResolverPresent and ValidateScopes become verifiers — they
    run post-compile and can safely reach into the domain without creating a
    cycle. Spark surfaces verifier errors as compile warnings, so
    AshGrant.Check.resolve_permissions/3 also raises a clear ArgumentError
    at authorization time if no resolver is configured.
  • Regression test. Adds test/ash_grant/code_interface_cycle_test.exs
    that pairs AshGrant on a resource with AshGrant.Domain +
    code_interface on the domain — this case deadlocked on the previous code.

Both fixes are authored against jhlee111/ash_grant@32c607d (the commit
team-alembic/ash_grant:main currently points at), so this PR applies
cleanly.

Test plan

  • mix test passes locally (includes the new code_interface_cycle_test.exs).
  • Exercised end-to-end by the ARCC Center codebase (Ash 3.x, 57 resources,
    domain-level code_interface across 11 domains, bypass policies for
    public registration on User, system-actor + superadmin wildcard grants).
    Full mix check (format, credo, compile --warnings-as-errors, 687
    tests) green with these two commits applied.

Notes

  • Happy to split into two separate PRs if that's preferred — they're
    independent. Kept them together here because both were blockers for the
    same adoption effort.
  • Commit e56b814 deletes lib/ash_grant/transformers/merge_domain_config.ex,
    lib/ash_grant/transformers/validate_resolver_present.ex, and
    lib/ash_grant/transformers/validate_scopes.ex; the replacement verifiers
    live under lib/ash_grant/verifiers/.

barnabasJ and others added 2 commits April 20, 2026 02:36
Ash bypass policies only short-circuit policies that come after them.
add_entity defaults to prepend, which placed AshGrant's generated
policies before user-defined bypass policies, making bypasses
ineffective.

Switch to type: :append so user-defined bypass policies are evaluated
first and can properly short-circuit AshGrant's permission checks.
…face

A resource using `AshGrant` plus a domain that uses `AshGrant.Domain` AND
declares a `code_interface do define … end` block deadlocked on compile:
the resource's `MergeDomainConfig` transformer forced the domain to compile,
while the domain's code_interface transformer was waiting for the resource.

Move domain config merging from a compile-time transformer to runtime
`AshGrant.Info` calls (`resolver/1` falls back to the domain, `scopes/1`
merges resource + domain scopes with resource precedence). Convert the
compile-time presence checks to Spark verifiers, which run post-compile
and can safely reach into the domain. Spark surfaces verifier errors as
compile warnings, so `AshGrant.Check.resolve_permissions/3` also raises a
clear `ArgumentError` when no resolver is configured at authorization time.

Adds a regression test (`code_interface_cycle_test.exs`) that pairs
`AshGrant` on a resource with `AshGrant.Domain` + `code_interface` on the
domain. This case deadlocked on the previous code.
C-Sinclair added a commit that referenced this pull request Apr 21, 2026
Addressing review items:

#1 (Important): GrantsResolver predicate eval no longer rescues silently.
  Any error is now logged at :warning with the resource and grant name;
  the grant is still treated as non-matching (fail-closed preserved).

#6 (Important): The @grants section DSL example had stale function-form
  predicates. Updated to use `expr(^actor(:role) == :admin)`, matching
  what the feature actually ships.

#7 (Important): Documented the known deny+sibling-allow limitation on
  `AshGrant.Dsl.Permission.@moduledoc`. The resolver emits the `!` prefix
  correctly and Evaluator honors deny-wins for broad allow + narrow deny,
  but narrow allow (:own) + narrow deny (:published) on the same action
  does not always surface in Check. Recommended workaround inline.

jhlee111#8 (Minor): Pulled through caller-provided `context.context` into
  `Ash.Expr.fill_template/2` so grant predicates can reference
  `^context(:key)` the same way scopes can.

jhlee111#10 (Minor): Tightened the `instance:` schema type from `:any` to
  `{:or, [{:in, [:*]}, :atom, :string]}`. Random structs and integers
  now fail at compile time instead of producing malformed permission
  strings at runtime.

Added a DSL test covering context threading.
@C-Sinclair
Copy link
Copy Markdown

Superseded by #4

@C-Sinclair C-Sinclair closed this Apr 21, 2026
C-Sinclair added a commit that referenced this pull request Apr 21, 2026
* feat: declarative grants DSL with compile-time verification

Adds a `grants` block to the `ash_grant` DSL so permissions can be
declared as data rather than returned from an imperative resolver
function. Each grant pairs an actor predicate with a set of named,
structured permissions and optional compliance-purpose metadata.

- `grant :name, predicate do ... end` — predicate is a 1-arity function
  over the actor; matching grants contribute their permissions to the
  synthesized resolver output
- `permission :name, :action, :scope` — positional mirrors the string
  form; `on:`, `instance:`, `purpose(s):`, `deny:`, `description:` as
  keyword options
- Extension-level `purposes [...]` declares the compliance vocabulary;
  unknown purpose atoms fail compilation
- `NormalizeGrants` transformer injects the enclosing resource as the
  default `on:` and rejects declaring both `grants` and an explicit
  `resolver`
- `ValidateGrantReferences` verifier checks that each permission's
  `on:` is an Ash.Resource, that `action:` exists on it (or is `:*`),
  and that `scope:` is defined in its `ash_grant` block
- `SynthesizeGrantsResolver` wires `AshGrant.GrantsResolver` as the
  resource's resolver when grants are declared — no other runtime
  machinery changes; Check/FilterCheck/Explainer pick it up unchanged
- `Info.grants/1`, `permissions/1`, `permissions_for_purpose/2`,
  `effective_purposes/2`, `declared_purposes/1` for compliance
  introspection

* refactor grants DSL: Ash.Expr predicates, drop purposes, DB integration tests

Following review feedback:

- Grant predicates are now `Ash.Expr` expressions evaluated via
  `Ash.Expr.eval/2` with `Ash.Expr.fill_template/2` for `^actor(:key)`
  substitution. The function form was inconsistent with how scopes and
  other AshGrant expressions already work.

      grant :admin, expr(^actor(:role) == :admin) do
        permission :manage_all, :*, :always
      end

- Removed `purpose` / `purposes` entirely from the DSL, schema, Info,
  NormalizeGrants, and tests. Compliance-purpose metadata needs deeper
  design before shipping.

- Added end-to-end integration test (`grants_integration_test.exs`) that
  exercises the full pipeline against AshPostgres and Ash.Policy.Authorizer:
  read filtering via FilterCheck, create/update enforcement via Check,
  :own scope narrowing, compound predicates, and actor/context edge cases.

- Added `AshGrant.Test.GrantsPost` test resource driven purely by the
  synthesized resolver, backed by a real `grants_posts` table.

- DSL test suite rewritten with `expr()` predicates; added compound and
  missing-field coverage.

* Address code-review feedback on grants DSL

Addressing review items:

#1 (Important): GrantsResolver predicate eval no longer rescues silently.
  Any error is now logged at :warning with the resource and grant name;
  the grant is still treated as non-matching (fail-closed preserved).

#6 (Important): The @grants section DSL example had stale function-form
  predicates. Updated to use `expr(^actor(:role) == :admin)`, matching
  what the feature actually ships.

#7 (Important): Documented the known deny+sibling-allow limitation on
  `AshGrant.Dsl.Permission.@moduledoc`. The resolver emits the `!` prefix
  correctly and Evaluator honors deny-wins for broad allow + narrow deny,
  but narrow allow (:own) + narrow deny (:published) on the same action
  does not always surface in Check. Recommended workaround inline.

jhlee111#8 (Minor): Pulled through caller-provided `context.context` into
  `Ash.Expr.fill_template/2` so grant predicates can reference
  `^context(:key)` the same way scopes can.

jhlee111#10 (Minor): Tightened the `instance:` schema type from `:any` to
  `{:or, [{:in, [:*]}, :atom, :string]}`. Random structs and integers
  now fail at compile time instead of producing malformed permission
  strings at runtime.

Added a DSL test covering context threading.

* mix format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants